diff --git a/Makefile b/Makefile index f216285..db271b8 100644 --- a/Makefile +++ b/Makefile @@ -511,6 +511,17 @@ release-docker: ## Build and push release Docker images pre-release-docker: ## Build and push pre-release Docker images ./publish-docker-prerelease.sh +dev-release: ## Build and push custom dev release (usage: make dev-release NAME=issue38) + @if [ -z "$(NAME)" ]; then \ + echo "$(RED)ERROR: NAME parameter is required!$(NC)"; \ + echo "$(YELLOW)Usage: make dev-release NAME=$(NC)"; \ + echo "$(YELLOW)Example: make dev-release NAME=issue38$(NC)"; \ + echo "$(YELLOW) This will create tags like: 0.3.1-dev-issue38$(NC)"; \ + exit 1; \ + fi + @echo "$(BLUE)Building custom dev release: $(NAME)$(NC)" + @./publish-docker-dev.sh $(NAME) + #=============================================================================== # DATABASE #=============================================================================== diff --git a/README.md b/README.md index 9c2510b..6b566c0 100644 --- a/README.md +++ b/README.md @@ -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) diff --git a/VERSION b/VERSION index 9e11b32..d15723f 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.3.1 +0.3.2 diff --git a/backend/package-lock.json b/backend/package-lock.json index c328369..77eb32a 100644 --- a/backend/package-lock.json +++ b/backend/package-lock.json @@ -1,12 +1,12 @@ { "name": "backend", - "version": "0.3.0", + "version": "0.3.2", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "backend", - "version": "0.3.0", + "version": "0.3.2", "license": "ISC", "dependencies": { "@prisma/client": "^5.22.0", diff --git a/backend/package.json b/backend/package.json index f75a26c..bb277ae 100644 --- a/backend/package.json +++ b/backend/package.json @@ -1,6 +1,6 @@ { "name": "backend", - "version": "0.3.1", + "version": "0.3.2", "description": "", "main": "index.js", "scripts": { diff --git a/backend/src/__tests__/csrf-trust-proxy.test.ts b/backend/src/__tests__/csrf-trust-proxy.test.ts new file mode 100644 index 0000000..beadca9 --- /dev/null +++ b/backend/src/__tests__/csrf-trust-proxy.test.ts @@ -0,0 +1,172 @@ +/** + * Issue #38: CSRF fails with multiple reverse proxies + * + * This test demonstrates how trust proxy settings affect CSRF validation + * when ExcaliDash is behind multiple proxy layers (e.g., Traefik, Synology NAS) + */ + +import { describe, it, expect, beforeEach, afterEach } from "vitest"; +import express from "express"; +import request from "supertest"; +import { + createCsrfToken, + validateCsrfToken, + getCsrfTokenHeader, +} from "../security"; + +// mock the getClientId function behavior +const getClientIdFromRequest = (req: express.Request): string => { + const ip = req.ip || req.connection.remoteAddress || "unknown"; + const userAgent = req.headers["user-agent"] || "unknown"; + return `${ip}:${userAgent}`.slice(0, 256); +}; + +describe("Issue #38: CSRF with trust proxy settings", () => { + let app: express.Application; + + beforeEach(() => { + app = express(); + app.use(express.json()); + }); + + it("demonstrates the trust proxy issue with multiple proxies", async () => { + // ext proxy -> frontend nginx -> backend + // X-Forwarded-For: 203.0.113.42 (client), 10.0.0.5 (external proxy), 172.17.0.3 (frontend nginx) + + // With trust proxy: 1 (current setting) + const app1 = express(); + app1.set("trust proxy", 1); + app1.use(express.json()); + + app1.get("/test-ip", (req, res) => { + res.json({ + ip: req.ip, + clientId: getClientIdFromRequest(req), + }); + }); + + // Simulate request through multiple proxies + const response1 = await request(app1) + .get("/test-ip") + .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 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, + "(not the real client IP)", + ); + + // With trust proxy: true + const app2 = express(); + app2.set("trust proxy", true); + app2.use(express.json()); + + app2.get("/test-ip", (req, res) => { + res.json({ + ip: req.ip, + clientId: getClientIdFromRequest(req), + }); + }); + + const response2 = await request(app2) + .get("/test-ip") + .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: true, Express takes leftmost IP + expect(response2.body.ip).toBe("203.0.113.42"); + console.log( + "trust proxy: true → IP:", + response2.body.ip, + "(real client IP - CORRECT)", + ); + }); + + it("simulates CSRF failure scenario from issue #38", async () => { + const userAgent = "Mozilla/5.0 (Windows NT 10.0; Win64; x64)"; + + // Request 1: Fetch CSRF token + // X-Forwarded-For shows: client, external-proxy-1, frontend-nginx + const clientIp1 = "203.0.113.42"; + const externalProxyIp1 = "10.0.0.5"; // External proxy IP on first request + + // With trust proxy: 1, Express sees the external proxy IP + const clientId1 = `${externalProxyIp1}:${userAgent}`; + const token = createCsrfToken(clientId1); + + console.log( + " X-Forwarded-For:", + `${clientIp1}, ${externalProxyIp1}, 172.17.0.3`, + ); + console.log(" Express sees IP:", externalProxyIp1); + console.log(" ClientId:", clientId1.slice(0, 50) + "..."); + + // Request 2: Try to create drawing with token + // External proxy IP might differ slightly + const externalProxyIp2 = "10.0.0.6"; + + const clientId2 = `${externalProxyIp2}:${userAgent}`; + + console.log( + " X-Forwarded-For:", + `${clientIp1}, ${externalProxyIp2}, 172.17.0.3`, + ); + console.log(" Express sees IP:", externalProxyIp2); + console.log(" ClientId:", clientId2.slice(0, 50) + "..."); + + // CSRF validation fails because clientId changed + const isValid = validateCsrfToken(clientId2, token); + + expect(isValid).toBe(false); + console.log(" Expected:", clientId1.slice(0, 50) + "..."); + console.log(" Got:", clientId2.slice(0, 50) + "..."); + }); + + it("shows the fix works with trust proxy: true", async () => { + const userAgent = "Mozilla/5.0 (Windows NT 10.0; Win64; x64)"; + const realClientIp = "203.0.113.42"; + + const clientId1 = `${realClientIp}:${userAgent}`; + const token = createCsrfToken(clientId1); + + console.log(" X-Forwarded-For:", `${realClientIp}, 10.0.0.5, 172.17.0.3`); + console.log(" Express sees IP:", realClientIp); + + // Request 2: Use token (even if middle proxy IPs differ) + const clientId2 = `${realClientIp}:${userAgent}`; + + console.log("Create drawing"); + console.log("X-Forwarded-For:", `${realClientIp}, 10.0.0.6, 172.17.0.3`); + console.log("Express sees IP:", realClientIp, "(same!)"); + + const isValid = validateCsrfToken(clientId2, token); + + expect(isValid).toBe(true); + console.log("\nCSRF Validation: SUCCESS"); + }); + + it("demonstrates the Synology NAS scenario from issue #38", async () => { + const app = express(); + app.set("trust proxy", 1); + app.use(express.json()); + + let seenIp: string | undefined; + app.get("/test", (req, res) => { + seenIp = req.ip; + res.json({ ip: req.ip }); + }); + + // 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.11.166"); // Not the real client IP + }); +}); diff --git a/backend/src/index.ts b/backend/src/index.ts index 6de2da1..506646e 100644 --- a/backend/src/index.ts +++ b/backend/src/index.ts @@ -148,8 +148,21 @@ const app = express(); // Trust proxy headers (X-Forwarded-For, X-Real-IP) from nginx // Required for correct client IP detection when running behind a reverse proxy -// This fixes CSRF token validation failures in Docker/K8s environments -app.set("trust proxy", 1); +// Fix for issue #38: Use 'true' to handle multiple proxy layers (e.g., Traefik, Synology NAS) +// This ensures Express extracts the real client IP from the leftmost X-Forwarded-For value +const trustProxyConfig = process.env.TRUST_PROXY || "true"; +const trustProxyValue = trustProxyConfig === "true" + ? true + : trustProxyConfig === "false" + ? false + : parseInt(trustProxyConfig, 10) || 1; +app.set("trust proxy", trustProxyValue); + +if (trustProxyValue === true) { + console.log("[config] trust proxy: enabled (handles multiple proxy layers)"); +} else { + console.log(`[config] trust proxy: ${trustProxyValue}`); +} const httpServer = createServer(app); const io = new Server(httpServer, { @@ -346,9 +359,24 @@ app.use((req, res, next) => { const getClientId = (req: express.Request): string => { const ip = req.ip || req.connection.remoteAddress || "unknown"; const userAgent = req.headers["user-agent"] || "unknown"; - // Create a simple hash for client identification - // In production, you might use a session ID instead - return `${ip}:${userAgent}`.slice(0, 256); + const clientId = `${ip}:${userAgent}`.slice(0, 256); + + // Debug logging for CSRF troubleshooting (issue #38) + if (process.env.DEBUG_CSRF === "true") { + console.log("[CSRF DEBUG] getClientId", { + method: req.method, + path: req.path, + ip, + remoteAddress: req.connection.remoteAddress, + "x-forwarded-for": req.headers["x-forwarded-for"], + "x-real-ip": req.headers["x-real-ip"], + userAgent: userAgent.slice(0, 100), + clientIdPreview: clientId.slice(0, 60) + "...", + trustProxySetting: req.app.get("trust proxy"), + }); + } + + return clientId; }; // Rate limiter specifically for CSRF token generation to prevent store exhaustion diff --git a/backend/src/security.ts b/backend/src/security.ts index 8696b11..3cb0771 100644 --- a/backend/src/security.ts +++ b/backend/src/security.ts @@ -583,8 +583,12 @@ const getCsrfSecret = (): Buffer => { cachedCsrfSecret = crypto.randomBytes(32); const envLabel = process.env.NODE_ENV ? ` (${process.env.NODE_ENV})` : ""; console.warn( - `[security] CSRF_SECRET is not set${envLabel}. Using an ephemeral per-process secret. ` + - "For horizontal scaling (k8s), set CSRF_SECRET to the same value on all instances." + `[SECURITY WARNING] CSRF_SECRET is not set${envLabel}.\n` + + `Using an ephemeral per-process secret.\n` + + ` - Tokens will expire on container restart\n` + + ` - Horizontal scaling (k8s) will NOT work\n` + + ` - Generate a secret: openssl rand -base64 32\n` + + ` - Set environment variable: CSRF_SECRET=` ); return cachedCsrfSecret; }; diff --git a/frontend/package.json b/frontend/package.json index c1f8eca..da2d6be 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -1,7 +1,7 @@ { "name": "frontend", "private": true, - "version": "0.3.1", + "version": "0.3.2", "type": "module", "scripts": { "dev": "vite", diff --git a/publish-docker-dev.sh b/publish-docker-dev.sh new file mode 100755 index 0000000..679d8a9 --- /dev/null +++ b/publish-docker-dev.sh @@ -0,0 +1,109 @@ +#!/bin/bash +set -e + +# Colors for output +RED='\033[0;31m' +GREEN='\033[0;32m' +YELLOW='\033[1;33m' +BLUE='\033[0;34m' +NC='\033[0m' # No Color + +# Custom name is required +CUSTOM_NAME=$1 + +if [ -z "$CUSTOM_NAME" ]; then + echo -e "${RED}ERROR: Custom name is required!${NC}" + echo -e "${YELLOW}Usage: $0 ${NC}" + echo -e "${YELLOW}Example: $0 issue38${NC}" + echo -e "${YELLOW} This will create tags like: 0.3.1-dev-issue38${NC}" + exit 1 +fi + +# Configuration +DOCKER_USERNAME="zimengxiong" +IMAGE_NAME="excalidash" +BASE_VERSION=$(node -e "try { console.log(require('fs').readFileSync('VERSION', 'utf8').trim()) } catch { console.log('0.0.0') }") +VERSION="${BASE_VERSION}-dev-${CUSTOM_NAME}" +CURRENT_BRANCH=$(git rev-parse --abbrev-ref HEAD) + +echo -e "${BLUE}===========================================${NC}" +echo -e "${BLUE}ExcaliDash Custom Dev Release${NC}" +echo -e "${BLUE}===========================================${NC}" +echo "" +echo -e "${YELLOW}Branch: ${CURRENT_BRANCH}${NC}" +echo -e "${YELLOW}Base version: ${BASE_VERSION}${NC}" +echo -e "${YELLOW}Custom name: ${CUSTOM_NAME}${NC}" +echo -e "${YELLOW}Full tag: ${VERSION}${NC}" +echo "" +echo -e "${YELLOW}This will publish images with tag: ${VERSION}${NC}" +echo -e "${YELLOW}Dev images will NOT update 'latest' or 'dev' tags${NC}" +echo "" + +# Confirm before proceeding +read -p "Continue? (y/N) " -n 1 -r +echo +if [[ ! $REPLY =~ ^[Yy]$ ]]; then + echo -e "${RED}Aborted.${NC}" + exit 1 +fi + +# Check if logged in to Docker Hub +echo -e "${YELLOW}Checking Docker Hub authentication...${NC}" +if ! docker info | grep -q "Username: $DOCKER_USERNAME"; then + echo -e "${YELLOW}Not logged in. Please login to Docker Hub:${NC}" + docker login +else + echo -e "${GREEN}✓ Already logged in as $DOCKER_USERNAME${NC}" +fi + +# Create buildx builder if it doesn't exist +echo -e "${YELLOW}Setting up buildx builder...${NC}" +if ! docker buildx inspect excalidash-builder > /dev/null 2>&1; then + echo -e "${YELLOW}Creating new buildx builder...${NC}" + docker buildx create --name excalidash-builder --use --bootstrap +else + echo -e "${GREEN}✓ Using existing buildx builder${NC}" + docker buildx use excalidash-builder +fi + +# Build and push backend image +echo "" +echo -e "${BLUE}Building and pushing backend image...${NC}" +docker buildx build \ + --platform linux/amd64,linux/arm64 \ + --tag $DOCKER_USERNAME/$IMAGE_NAME-backend:$VERSION \ + --file backend/Dockerfile \ + --push \ + backend/ + +echo -e "${GREEN}✓ Backend image pushed successfully${NC}" + +# Build and push frontend image +echo "" +echo -e "${BLUE}Building and pushing frontend image...${NC}" +docker buildx build \ + --platform linux/amd64,linux/arm64 \ + --tag $DOCKER_USERNAME/$IMAGE_NAME-frontend:$VERSION \ + --build-arg VITE_APP_VERSION=$VERSION \ + --file frontend/Dockerfile \ + --push \ + . + +echo -e "${GREEN}✓ Frontend image pushed successfully${NC}" + +echo "" +echo -e "${BLUE}===========================================${NC}" +echo -e "${GREEN}✓ Custom dev images published!${NC}" +echo -e "${BLUE}===========================================${NC}" +echo "" +echo -e "${YELLOW}Images published:${NC}" +echo -e " • $DOCKER_USERNAME/$IMAGE_NAME-backend:$VERSION" +echo -e " • $DOCKER_USERNAME/$IMAGE_NAME-frontend:$VERSION" +echo "" +echo -e "${YELLOW}To use these images in docker-compose:${NC}" +echo -e "${BLUE} services:" +echo -e " backend:" +echo -e " image: $DOCKER_USERNAME/$IMAGE_NAME-backend:$VERSION" +echo -e " frontend:" +echo -e " image: $DOCKER_USERNAME/$IMAGE_NAME-frontend:$VERSION${NC}" +echo ""