Compare commits

..

4 Commits

Author SHA1 Message Date
Zimeng Xiong e9d349bb0e fix express proxy headers 2026-01-30 14:28:38 -08:00
Zimeng Xiong 6a84cc4ab7 repro issue 2026-01-30 14:19:24 -08:00
Zimeng Xiong 81918b00cd chore: release v0.3.1 2026-01-20 13:41:22 -08:00
Zimeng Xiong 3b384dc5fb CSRF token validation failing behind nginx proxy (#38)
Express was not configured to trust proxy headers, causing req.ip to return nginx's internal container IP instead of the actual client IP. In Docker environments, nginx can appear with different internal IPs between requests, causing the CSRF clientId to change and token validation to fail.
2026-01-20 13:39:33 -08:00
10 changed files with 348 additions and 42 deletions
+11
View File
@@ -511,6 +511,17 @@ release-docker: ## Build and push release Docker images
pre-release-docker: ## Build and push pre-release Docker images pre-release-docker: ## Build and push pre-release Docker images
./publish-docker-prerelease.sh ./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=<custom-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 # DATABASE
#=============================================================================== #===============================================================================
+1 -1
View File
@@ -1 +1 @@
0.3.0 0.3.1
+2 -8
View File
@@ -1,12 +1,12 @@
{ {
"name": "backend", "name": "backend",
"version": "0.1.8", "version": "0.3.1",
"lockfileVersion": 3, "lockfileVersion": 3,
"requires": true, "requires": true,
"packages": { "packages": {
"": { "": {
"name": "backend", "name": "backend",
"version": "0.1.8", "version": "0.3.1",
"license": "ISC", "license": "ISC",
"dependencies": { "dependencies": {
"@prisma/client": "^5.22.0", "@prisma/client": "^5.22.0",
@@ -1128,7 +1128,6 @@
"resolved": "https://registry.npmjs.org/@types/node/-/node-24.10.1.tgz", "resolved": "https://registry.npmjs.org/@types/node/-/node-24.10.1.tgz",
"integrity": "sha512-GNWcUTRBgIRJD5zj+Tq0fKOJ5XZajIiBroOF0yvj2bSU1WvNdYS/dn9UxwsujGW4JX06dnHyjV2y9rRaybH0iQ==", "integrity": "sha512-GNWcUTRBgIRJD5zj+Tq0fKOJ5XZajIiBroOF0yvj2bSU1WvNdYS/dn9UxwsujGW4JX06dnHyjV2y9rRaybH0iQ==",
"license": "MIT", "license": "MIT",
"peer": true,
"dependencies": { "dependencies": {
"undici-types": "~7.16.0" "undici-types": "~7.16.0"
} }
@@ -3814,7 +3813,6 @@
"integrity": "sha512-vtpjW3XuYCSnMsNVBjLMNkTj6OZbudcPPTPYHqX0CJfpcdWciI1dM8uHETwmDxxiqEwCIE6WvXucWUetJgfu/A==", "integrity": "sha512-vtpjW3XuYCSnMsNVBjLMNkTj6OZbudcPPTPYHqX0CJfpcdWciI1dM8uHETwmDxxiqEwCIE6WvXucWUetJgfu/A==",
"hasInstallScript": true, "hasInstallScript": true,
"license": "Apache-2.0", "license": "Apache-2.0",
"peer": true,
"dependencies": { "dependencies": {
"@prisma/engines": "5.22.0" "@prisma/engines": "5.22.0"
}, },
@@ -4821,7 +4819,6 @@
"integrity": "sha512-5gTmgEY/sqK6gFXLIsQNH19lWb4ebPDLA4SdLP7dsWkIXHWlG66oPuVvXSGFPppYZz8ZDZq0dYYrbHfBCVUb1Q==", "integrity": "sha512-5gTmgEY/sqK6gFXLIsQNH19lWb4ebPDLA4SdLP7dsWkIXHWlG66oPuVvXSGFPppYZz8ZDZq0dYYrbHfBCVUb1Q==",
"dev": true, "dev": true,
"license": "MIT", "license": "MIT",
"peer": true,
"engines": { "engines": {
"node": ">=12" "node": ">=12"
}, },
@@ -4980,7 +4977,6 @@
"integrity": "sha512-jl1vZzPDinLr9eUt3J/t7V6FgNEw9QjvBPdysz9KfQDD41fQrC2Y4vKQdiaUpFT4bXlb1RHhLpp8wtm6M5TgSw==", "integrity": "sha512-jl1vZzPDinLr9eUt3J/t7V6FgNEw9QjvBPdysz9KfQDD41fQrC2Y4vKQdiaUpFT4bXlb1RHhLpp8wtm6M5TgSw==",
"dev": true, "dev": true,
"license": "Apache-2.0", "license": "Apache-2.0",
"peer": true,
"bin": { "bin": {
"tsc": "bin/tsc", "tsc": "bin/tsc",
"tsserver": "bin/tsserver" "tsserver": "bin/tsserver"
@@ -5058,7 +5054,6 @@
"integrity": "sha512-tI2l/nFHC5rLh7+5+o7QjKjSR04ivXDF4jcgV0f/bTQ+OJiITy5S6gaynVsEM+7RqzufMnVbIon6Sr5x1SDYaQ==", "integrity": "sha512-tI2l/nFHC5rLh7+5+o7QjKjSR04ivXDF4jcgV0f/bTQ+OJiITy5S6gaynVsEM+7RqzufMnVbIon6Sr5x1SDYaQ==",
"dev": true, "dev": true,
"license": "MIT", "license": "MIT",
"peer": true,
"dependencies": { "dependencies": {
"esbuild": "^0.25.0", "esbuild": "^0.25.0",
"fdir": "^6.5.0", "fdir": "^6.5.0",
@@ -5152,7 +5147,6 @@
"integrity": "sha512-5gTmgEY/sqK6gFXLIsQNH19lWb4ebPDLA4SdLP7dsWkIXHWlG66oPuVvXSGFPppYZz8ZDZq0dYYrbHfBCVUb1Q==", "integrity": "sha512-5gTmgEY/sqK6gFXLIsQNH19lWb4ebPDLA4SdLP7dsWkIXHWlG66oPuVvXSGFPppYZz8ZDZq0dYYrbHfBCVUb1Q==",
"dev": true, "dev": true,
"license": "MIT", "license": "MIT",
"peer": true,
"engines": { "engines": {
"node": ">=12" "node": ">=12"
}, },
+1 -1
View File
@@ -1,6 +1,6 @@
{ {
"name": "backend", "name": "backend",
"version": "0.3.0", "version": "0.3.1",
"description": "", "description": "",
"main": "index.js", "main": "index.js",
"scripts": { "scripts": {
@@ -0,0 +1,169 @@
/**
* 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, Express takes second-to-last IP (the external proxy)
expect(response1.body.ip).toBe("10.0.0.5");
console.log(
"trust proxy: 1 → IP:",
response1.body.ip,
"(external proxy IP - WRONG)",
);
// 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
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
});
});
+37 -3
View File
@@ -129,6 +129,25 @@ const initializeUploadDir = async () => {
}; };
const app = express(); 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
// 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 httpServer = createServer(app);
const io = new Server(httpServer, { const io = new Server(httpServer, {
cors: { cors: {
@@ -324,9 +343,24 @@ app.use((req, res, next) => {
const getClientId = (req: express.Request): string => { const getClientId = (req: express.Request): string => {
const ip = req.ip || req.connection.remoteAddress || "unknown"; const ip = req.ip || req.connection.remoteAddress || "unknown";
const userAgent = req.headers["user-agent"] || "unknown"; const userAgent = req.headers["user-agent"] || "unknown";
// Create a simple hash for client identification const clientId = `${ip}:${userAgent}`.slice(0, 256);
// In production, you might use a session ID instead
return `${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 // Rate limiter specifically for CSRF token generation to prevent store exhaustion
+15 -25
View File
@@ -30,9 +30,7 @@ let activeConfig: SecurityConfig = { ...defaultConfig };
* Configure security settings * Configure security settings
* @param config Partial configuration to merge with defaults * @param config Partial configuration to merge with defaults
*/ */
export const configureSecuritySettings = ( export const configureSecuritySettings = (config: Partial<SecurityConfig>): void => {
config: Partial<SecurityConfig>
): void => {
activeConfig = { ...activeConfig, ...config }; activeConfig = { ...activeConfig, ...config };
}; };
@@ -320,13 +318,10 @@ export const appStateSchema = z
.optional() .optional()
.nullable(), .nullable(),
currentItemRoundness: z currentItemRoundness: z
.union([ .object({
z.enum(["sharp", "round"]),
z.object({
type: z.enum(["round", "sharp"]), type: z.enum(["round", "sharp"]),
value: z.number().finite().min(0).max(1), value: z.number().finite().min(0).max(1),
}), })
])
.optional() .optional()
.nullable(), .nullable(),
currentItemFontSize: z currentItemFontSize: z
@@ -432,19 +427,10 @@ export const sanitizeDrawingData = (data: {
]; ];
// Dangerous URL protocols to block entirely // Dangerous URL protocols to block entirely
const dangerousProtocols = [ const dangerousProtocols = [/^javascript:/i, /^vbscript:/i, /^data:text\/html/i];
/^javascript:/i,
/^vbscript:/i,
/^data:text\/html/i,
];
// Suspicious patterns for security validation within data URLs // Suspicious patterns for security validation within data URLs
const suspiciousPatterns = [ const suspiciousPatterns = [/<script/i, /javascript:/i, /on\w+\s*=/i, /<iframe/i];
/<script/i,
/javascript:/i,
/on\w+\s*=/i,
/<iframe/i,
];
// Maximum size for dataURL (configurable, default 10MB to prevent DoS) // Maximum size for dataURL (configurable, default 10MB to prevent DoS)
const MAX_DATAURL_SIZE = activeConfig.maxDataUrlSize; const MAX_DATAURL_SIZE = activeConfig.maxDataUrlSize;
@@ -462,8 +448,8 @@ export const sanitizeDrawingData = (data: {
const normalizedValue = value.toLowerCase(); const normalizedValue = value.toLowerCase();
// First, check for dangerous protocols - block these entirely // First, check for dangerous protocols - block these entirely
const hasDangerousProtocol = dangerousProtocols.some( const hasDangerousProtocol = dangerousProtocols.some((pattern) =>
(pattern) => pattern.test(value) pattern.test(value)
); );
if (hasDangerousProtocol) { if (hasDangerousProtocol) {
@@ -479,8 +465,8 @@ export const sanitizeDrawingData = (data: {
if (isSafeImageType) { if (isSafeImageType) {
// Check for suspicious content and size limits // Check for suspicious content and size limits
const hasSuspiciousContent = suspiciousPatterns.some( const hasSuspiciousContent = suspiciousPatterns.some((pattern) =>
(pattern) => pattern.test(value) pattern.test(value)
); );
const isTooLarge = value.length > MAX_DATAURL_SIZE; const isTooLarge = value.length > MAX_DATAURL_SIZE;
@@ -583,8 +569,12 @@ const getCsrfSecret = (): Buffer => {
cachedCsrfSecret = crypto.randomBytes(32); cachedCsrfSecret = crypto.randomBytes(32);
const envLabel = process.env.NODE_ENV ? ` (${process.env.NODE_ENV})` : ""; const envLabel = process.env.NODE_ENV ? ` (${process.env.NODE_ENV})` : "";
console.warn( console.warn(
`[security] CSRF_SECRET is not set${envLabel}. Using an ephemeral per-process secret. ` + `[SECURITY WARNING] CSRF_SECRET is not set${envLabel}.\n` +
"For horizontal scaling (k8s), set CSRF_SECRET to the same value on all instances." `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=<generated-secret>`
); );
return cachedCsrfSecret; return cachedCsrfSecret;
}; };
+1 -1
View File
@@ -1,7 +1,7 @@
{ {
"name": "frontend", "name": "frontend",
"private": true, "private": true,
"version": "0.3.0", "version": "0.3.1",
"type": "module", "type": "module",
"scripts": { "scripts": {
"dev": "vite", "dev": "vite",
-1
View File
@@ -301,7 +301,6 @@ export const Editor: React.FC = () => {
try { try {
const persistableAppState = { const persistableAppState = {
...appState,
viewBackgroundColor: appState?.viewBackgroundColor || '#ffffff', viewBackgroundColor: appState?.viewBackgroundColor || '#ffffff',
gridSize: appState?.gridSize || null, gridSize: appState?.gridSize || null,
}; };
+109
View File
@@ -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 <custom-name>${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 ""