From 5f18c06e03695b2567ccd802c830d6533d825773 Mon Sep 17 00:00:00 2001 From: Shreyas Date: Mon, 23 Feb 2026 12:25:24 +0530 Subject: [PATCH 1/4] fix: use collision-free path encoding for Traefik router key generation --- server/lib/traefik/getTraefikConfig.ts | 6 +- server/lib/traefik/pathEncoding.test.ts | 170 ++++++++++++++++++ server/lib/traefik/utils.ts | 20 +++ .../private/lib/traefik/getTraefikConfig.ts | 8 +- 4 files changed, 199 insertions(+), 5 deletions(-) create mode 100644 server/lib/traefik/pathEncoding.test.ts diff --git a/server/lib/traefik/getTraefikConfig.ts b/server/lib/traefik/getTraefikConfig.ts index 06754ffa2..f503b88a3 100644 --- a/server/lib/traefik/getTraefikConfig.ts +++ b/server/lib/traefik/getTraefikConfig.ts @@ -14,7 +14,7 @@ import logger from "@server/logger"; import config from "@server/lib/config"; import { resources, sites, Target, targets } from "@server/db"; import createPathRewriteMiddleware from "./middleware"; -import { sanitize, validatePathRewriteConfig } from "./utils"; +import { sanitize, encodePath, validatePathRewriteConfig } from "./utils"; const redirectHttpsMiddlewareName = "redirect-to-https"; const badgerMiddlewareName = "badger"; @@ -44,7 +44,7 @@ export async function getTraefikConfig( filterOutNamespaceDomains = false, // UNUSED BUT USED IN PRIVATE generateLoginPageRouters = false, // UNUSED BUT USED IN PRIVATE allowRawResources = true, - allowMaintenancePage = true, // UNUSED BUT USED IN PRIVATE + allowMaintenancePage = true // UNUSED BUT USED IN PRIVATE ): Promise { // Get resources with their targets and sites in a single optimized query // Start from sites on this exit node, then join to targets and resources @@ -127,7 +127,7 @@ export async function getTraefikConfig( resourcesWithTargetsAndSites.forEach((row) => { const resourceId = row.resourceId; const resourceName = sanitize(row.resourceName) || ""; - const targetPath = sanitize(row.path) || ""; // Handle null/undefined paths + const targetPath = encodePath(row.path); // Encode path preserving uniqueness for key generation const pathMatchType = row.pathMatchType || ""; const rewritePath = row.rewritePath || ""; const rewritePathType = row.rewritePathType || ""; diff --git a/server/lib/traefik/pathEncoding.test.ts b/server/lib/traefik/pathEncoding.test.ts new file mode 100644 index 000000000..7e6d9266f --- /dev/null +++ b/server/lib/traefik/pathEncoding.test.ts @@ -0,0 +1,170 @@ +import { assertEquals } from "@test/assert"; +import { encodePath, sanitize } from "./utils"; + +function runTests() { + console.log("Running path encoding tests...\n"); + + // Test 1: null and empty return empty string + { + assertEquals(encodePath(null), "", "null should return empty"); + assertEquals( + encodePath(undefined), + "", + "undefined should return empty" + ); + assertEquals(encodePath(""), "", "empty string should return empty"); + console.log(" PASS: null/undefined/empty return empty string"); + } + + // Test 2: root path "/" encodes to something non-empty + { + const result = encodePath("/"); + assertEquals(result !== "", true, "root path should not be empty"); + assertEquals(result, "2f", "root path should encode to hex of '/'"); + console.log(" PASS: root path encodes to non-empty string"); + } + + // Test 3: different paths produce different encoded values + { + const paths = [ + "/", + "/api", + "/a/b", + "/a-b", + "/a.b", + "/a_b", + "/api/v1", + "/api/v2" + ]; + const encoded = new Set(); + let collision = false; + for (const p of paths) { + const e = encodePath(p); + if (encoded.has(e)) { + collision = true; + break; + } + encoded.add(e); + } + assertEquals(collision, false, "no two different paths should collide"); + console.log(" PASS: all different paths produce unique encodings"); + } + + // Test 4: alphanumeric characters pass through unchanged + { + assertEquals( + encodePath("/api"), + "2fapi", + "/api should encode slash only" + ); + assertEquals(encodePath("/v1"), "2fv1", "/v1 should encode slash only"); + console.log(" PASS: alphanumeric characters preserved"); + } + + // Test 5: special characters are hex-encoded + { + const dotEncoded = encodePath("/a.b"); + const dashEncoded = encodePath("/a-b"); + const slashEncoded = encodePath("/a/b"); + const underscoreEncoded = encodePath("/a_b"); + + // all should be different + const set = new Set([ + dotEncoded, + dashEncoded, + slashEncoded, + underscoreEncoded + ]); + assertEquals( + set.size, + 4, + "dot, dash, slash, underscore paths should all be unique" + ); + console.log(" PASS: special characters produce unique encodings"); + } + + // Test 6: full key generation - different paths create different keys + { + function makeKey( + resourceId: number, + path: string | null, + pathMatchType: string | null + ) { + const targetPath = encodePath(path); + const pmt = pathMatchType || ""; + const pathKey = [targetPath, pmt, "", ""].filter(Boolean).join("-"); + const mapKey = [resourceId, pathKey].filter(Boolean).join("-"); + return sanitize(mapKey); + } + + const keySlash = makeKey(1, "/", "prefix"); + const keyApi = makeKey(1, "/api", "prefix"); + const keyNull = makeKey(1, null, null); + + assertEquals( + keySlash !== keyApi, + true, + "/ and /api should have different keys" + ); + assertEquals( + keySlash !== keyNull, + true, + "/ and null should have different keys" + ); + assertEquals( + keyApi !== keyNull, + true, + "/api and null should have different keys" + ); + + console.log( + " PASS: different paths create different resource map keys" + ); + } + + // Test 7: same path always produces same key (deterministic) + { + assertEquals( + encodePath("/api"), + encodePath("/api"), + "same input should produce same output" + ); + assertEquals( + encodePath("/a/b/c"), + encodePath("/a/b/c"), + "same input should produce same output" + ); + console.log(" PASS: encoding is deterministic"); + } + + // Test 8: encoded result is alphanumeric (valid for Traefik names after sanitize) + { + const paths = [ + "/", + "/api", + "/a/b", + "/a-b", + "/a.b", + "/complex/path/here" + ]; + for (const p of paths) { + const e = encodePath(p); + const isAlphanumeric = /^[a-zA-Z0-9]+$/.test(e); + assertEquals( + isAlphanumeric, + true, + `encodePath("${p}") = "${e}" should be alphanumeric` + ); + } + console.log(" PASS: encoded values are alphanumeric"); + } + + console.log("\nAll path encoding tests passed!"); +} + +try { + runTests(); +} catch (error) { + console.error("Test failed:", error); + process.exit(1); +} diff --git a/server/lib/traefik/utils.ts b/server/lib/traefik/utils.ts index ec0eae5b3..da876fa07 100644 --- a/server/lib/traefik/utils.ts +++ b/server/lib/traefik/utils.ts @@ -13,6 +13,26 @@ export function sanitize(input: string | null | undefined): string | undefined { .replace(/^-|-$/g, ""); } +/** + * Encode a URL path into a collision-free alphanumeric string suitable for use + * in Traefik router/service names and map keys. + * + * Unlike sanitize(), this preserves uniqueness by encoding each non-alphanumeric + * character as its hex code. Different paths always produce different outputs. + * + * encodePath("/api") => "2fapi" + * encodePath("/a/b") => "2fa2fb" + * encodePath("/a-b") => "2fa2db" (different from /a/b) + * encodePath("/") => "2f" + * encodePath(null) => "" + */ +export function encodePath(path: string | null | undefined): string { + if (!path) return ""; + return path.replace(/[^a-zA-Z0-9]/g, (ch) => { + return ch.charCodeAt(0).toString(16); + }); +} + export function validatePathRewriteConfig( path: string | null, pathMatchType: string | null, diff --git a/server/private/lib/traefik/getTraefikConfig.ts b/server/private/lib/traefik/getTraefikConfig.ts index f0343c5d4..1652427b2 100644 --- a/server/private/lib/traefik/getTraefikConfig.ts +++ b/server/private/lib/traefik/getTraefikConfig.ts @@ -34,7 +34,11 @@ import { import logger from "@server/logger"; import config from "@server/lib/config"; import { orgs, resources, sites, Target, targets } from "@server/db"; -import { sanitize, validatePathRewriteConfig } from "@server/lib/traefik/utils"; +import { + sanitize, + encodePath, + validatePathRewriteConfig +} from "@server/lib/traefik/utils"; import privateConfig from "#private/lib/config"; import createPathRewriteMiddleware from "@server/lib/traefik/middleware"; import { @@ -170,7 +174,7 @@ export async function getTraefikConfig( resourcesWithTargetsAndSites.forEach((row) => { const resourceId = row.resourceId; const resourceName = sanitize(row.resourceName) || ""; - const targetPath = sanitize(row.path) || ""; // Handle null/undefined paths + const targetPath = encodePath(row.path); // Encode path preserving uniqueness for key generation const pathMatchType = row.pathMatchType || ""; const rewritePath = row.rewritePath || ""; const rewritePathType = row.rewritePathType || ""; From e58f0c9f07cafe58b33e2f2b718709f528d1900c Mon Sep 17 00:00:00 2001 From: Shreyas Date: Mon, 23 Feb 2026 12:31:30 +0530 Subject: [PATCH 2/4] fix: preserve backward-compatible router names while fixing path collisions Use encodePath only for internal map key grouping (collision-free) and sanitize for Traefik-facing router/service names (unchanged for existing users). Extract pure functions into pathUtils.ts so tests can run without DB dependencies. --- server/lib/traefik/getTraefikConfig.ts | 39 ++++++++--- server/lib/traefik/pathEncoding.test.ts | 70 ++++++++++++++++++- server/lib/traefik/pathUtils.ts | 37 ++++++++++ server/lib/traefik/utils.ts | 34 +-------- .../private/lib/traefik/getTraefikConfig.ts | 39 ++++++++--- 5 files changed, 165 insertions(+), 54 deletions(-) create mode 100644 server/lib/traefik/pathUtils.ts diff --git a/server/lib/traefik/getTraefikConfig.ts b/server/lib/traefik/getTraefikConfig.ts index f503b88a3..3becfb370 100644 --- a/server/lib/traefik/getTraefikConfig.ts +++ b/server/lib/traefik/getTraefikConfig.ts @@ -127,25 +127,42 @@ export async function getTraefikConfig( resourcesWithTargetsAndSites.forEach((row) => { const resourceId = row.resourceId; const resourceName = sanitize(row.resourceName) || ""; - const targetPath = encodePath(row.path); // Encode path preserving uniqueness for key generation const pathMatchType = row.pathMatchType || ""; const rewritePath = row.rewritePath || ""; const rewritePathType = row.rewritePathType || ""; const priority = row.priority ?? 100; - // Create a unique key combining resourceId, path config, and rewrite config - const pathKey = [ - targetPath, + // Use encodePath for the internal map key to avoid collisions + // (e.g. "/a/b" vs "/a-b" must map to different groups) + const encodedPath = encodePath(row.path); + const internalPathKey = [ + encodedPath, pathMatchType, rewritePath, rewritePathType ] .filter(Boolean) .join("-"); - const mapKey = [resourceId, pathKey].filter(Boolean).join("-"); - const key = sanitize(mapKey); + const internalMapKey = [resourceId, internalPathKey] + .filter(Boolean) + .join("-"); - if (!resourcesMap.has(key)) { + // Use sanitize for the Traefik-facing key to preserve backward-compatible + // router/service names (existing sticky session cookies, etc.) + const sanitizedPath = sanitize(row.path) || ""; + const traefikPathKey = [ + sanitizedPath, + pathMatchType, + rewritePath, + rewritePathType + ] + .filter(Boolean) + .join("-"); + const traefikKey = sanitize( + [resourceId, traefikPathKey].filter(Boolean).join("-") + ); + + if (!resourcesMap.has(internalMapKey)) { const validation = validatePathRewriteConfig( row.path, row.pathMatchType, @@ -160,9 +177,10 @@ export async function getTraefikConfig( return; } - resourcesMap.set(key, { + resourcesMap.set(internalMapKey, { resourceId: row.resourceId, name: resourceName, + traefikKey: traefikKey, // backward-compatible key for Traefik names fullDomain: row.fullDomain, ssl: row.ssl, http: row.http, @@ -190,7 +208,7 @@ export async function getTraefikConfig( }); } - resourcesMap.get(key).targets.push({ + resourcesMap.get(internalMapKey).targets.push({ resourceId: row.resourceId, targetId: row.targetId, ip: row.ip, @@ -227,8 +245,9 @@ export async function getTraefikConfig( }; // get the key and the resource - for (const [key, resource] of resourcesMap.entries()) { + for (const [, resource] of resourcesMap.entries()) { const targets = resource.targets as TargetWithSite[]; + const key = resource.traefikKey; // backward-compatible key for Traefik names const routerName = `${key}-${resource.name}-router`; const serviceName = `${key}-${resource.name}-service`; diff --git a/server/lib/traefik/pathEncoding.test.ts b/server/lib/traefik/pathEncoding.test.ts index 7e6d9266f..a3575b9f2 100644 --- a/server/lib/traefik/pathEncoding.test.ts +++ b/server/lib/traefik/pathEncoding.test.ts @@ -1,5 +1,5 @@ -import { assertEquals } from "@test/assert"; -import { encodePath, sanitize } from "./utils"; +import { assertEquals } from "../../../test/assert"; +import { encodePath, sanitize } from "./pathUtils"; function runTests() { console.log("Running path encoding tests...\n"); @@ -159,6 +159,72 @@ function runTests() { console.log(" PASS: encoded values are alphanumeric"); } + // Test 9: backward compatibility - Traefik names use sanitize, internal keys use encodePath + { + // Simulate the dual-key approach from getTraefikConfig + function makeKeys( + resourceId: number, + path: string | null, + pathMatchType: string | null + ) { + // Internal map key (collision-free grouping) + const encodedPath = encodePath(path); + const internalPathKey = [encodedPath, pathMatchType || ""] + .filter(Boolean) + .join("-"); + const internalMapKey = [resourceId, internalPathKey] + .filter(Boolean) + .join("-"); + + // Traefik-facing key (backward-compatible naming) + const sanitizedPath = sanitize(path) || ""; + const traefikPathKey = [sanitizedPath, pathMatchType || ""] + .filter(Boolean) + .join("-"); + const traefikKey = sanitize( + [resourceId, traefikPathKey].filter(Boolean).join("-") + ); + + return { internalMapKey, traefikKey }; + } + + // /a/b and /a-b should have DIFFERENT internal keys (no collision) + const keysAB = makeKeys(1, "/a/b", "prefix"); + const keysDash = makeKeys(1, "/a-b", "prefix"); + assertEquals( + keysAB.internalMapKey !== keysDash.internalMapKey, + true, + "/a/b and /a-b must have different internal map keys" + ); + + // /a/b and /a-b produce the SAME Traefik key (backward compat — sanitize maps both to "a-b") + assertEquals( + keysAB.traefikKey, + keysDash.traefikKey, + "/a/b and /a-b should produce same Traefik key (sanitize behavior)" + ); + + // For a resource with no path, Traefik key should match the old behavior + const keysNoPath = makeKeys(42, null, null); + assertEquals( + keysNoPath.traefikKey, + "42", + "no-path resource should have resourceId-only Traefik key" + ); + + // Traefik key for /api prefix should match old sanitize-based output + const keysApi = makeKeys(1, "/api", "prefix"); + assertEquals( + keysApi.traefikKey, + "1-api-prefix", + "Traefik key should match old sanitize-based format" + ); + + console.log( + " PASS: backward compatibility - internal keys differ, Traefik names preserved" + ); + } + console.log("\nAll path encoding tests passed!"); } diff --git a/server/lib/traefik/pathUtils.ts b/server/lib/traefik/pathUtils.ts new file mode 100644 index 000000000..1b9d57a89 --- /dev/null +++ b/server/lib/traefik/pathUtils.ts @@ -0,0 +1,37 @@ +/** + * Pure utility functions for path/name encoding. + * No external dependencies — safe to import in tests. + */ + +export function sanitize(input: string | null | undefined): string | undefined { + if (!input) return undefined; + // clean any non alphanumeric characters from the input and replace with dashes + // the input cant be too long either, so limit to 50 characters + if (input.length > 50) { + input = input.substring(0, 50); + } + return input + .replace(/[^a-zA-Z0-9-]/g, "-") + .replace(/-+/g, "-") + .replace(/^-|-$/g, ""); +} + +/** + * Encode a URL path into a collision-free alphanumeric string suitable for use + * in Traefik map keys. + * + * Unlike sanitize(), this preserves uniqueness by encoding each non-alphanumeric + * character as its hex code. Different paths always produce different outputs. + * + * encodePath("/api") => "2fapi" + * encodePath("/a/b") => "2fa2fb" + * encodePath("/a-b") => "2fa2db" (different from /a/b) + * encodePath("/") => "2f" + * encodePath(null) => "" + */ +export function encodePath(path: string | null | undefined): string { + if (!path) return ""; + return path.replace(/[^a-zA-Z0-9]/g, (ch) => { + return ch.charCodeAt(0).toString(16); + }); +} diff --git a/server/lib/traefik/utils.ts b/server/lib/traefik/utils.ts index da876fa07..f40e2eba2 100644 --- a/server/lib/traefik/utils.ts +++ b/server/lib/traefik/utils.ts @@ -1,37 +1,7 @@ import logger from "@server/logger"; -export function sanitize(input: string | null | undefined): string | undefined { - if (!input) return undefined; - // clean any non alphanumeric characters from the input and replace with dashes - // the input cant be too long either, so limit to 50 characters - if (input.length > 50) { - input = input.substring(0, 50); - } - return input - .replace(/[^a-zA-Z0-9-]/g, "-") - .replace(/-+/g, "-") - .replace(/^-|-$/g, ""); -} - -/** - * Encode a URL path into a collision-free alphanumeric string suitable for use - * in Traefik router/service names and map keys. - * - * Unlike sanitize(), this preserves uniqueness by encoding each non-alphanumeric - * character as its hex code. Different paths always produce different outputs. - * - * encodePath("/api") => "2fapi" - * encodePath("/a/b") => "2fa2fb" - * encodePath("/a-b") => "2fa2db" (different from /a/b) - * encodePath("/") => "2f" - * encodePath(null) => "" - */ -export function encodePath(path: string | null | undefined): string { - if (!path) return ""; - return path.replace(/[^a-zA-Z0-9]/g, (ch) => { - return ch.charCodeAt(0).toString(16); - }); -} +// Re-export pure functions from dependency-free module +export { sanitize, encodePath } from "./pathUtils"; export function validatePathRewriteConfig( path: string | null, diff --git a/server/private/lib/traefik/getTraefikConfig.ts b/server/private/lib/traefik/getTraefikConfig.ts index 1652427b2..b6c460072 100644 --- a/server/private/lib/traefik/getTraefikConfig.ts +++ b/server/private/lib/traefik/getTraefikConfig.ts @@ -174,7 +174,6 @@ export async function getTraefikConfig( resourcesWithTargetsAndSites.forEach((row) => { const resourceId = row.resourceId; const resourceName = sanitize(row.resourceName) || ""; - const targetPath = encodePath(row.path); // Encode path preserving uniqueness for key generation const pathMatchType = row.pathMatchType || ""; const rewritePath = row.rewritePath || ""; const rewritePathType = row.rewritePathType || ""; @@ -184,19 +183,37 @@ export async function getTraefikConfig( return; } - // Create a unique key combining resourceId, path config, and rewrite config - const pathKey = [ - targetPath, + // Use encodePath for the internal map key to avoid collisions + // (e.g. "/a/b" vs "/a-b" must map to different groups) + const encodedPath = encodePath(row.path); + const internalPathKey = [ + encodedPath, pathMatchType, rewritePath, rewritePathType ] .filter(Boolean) .join("-"); - const mapKey = [resourceId, pathKey].filter(Boolean).join("-"); - const key = sanitize(mapKey); + const internalMapKey = [resourceId, internalPathKey] + .filter(Boolean) + .join("-"); - if (!resourcesMap.has(key)) { + // Use sanitize for the Traefik-facing key to preserve backward-compatible + // router/service names (existing sticky session cookies, etc.) + const sanitizedPath = sanitize(row.path) || ""; + const traefikPathKey = [ + sanitizedPath, + pathMatchType, + rewritePath, + rewritePathType + ] + .filter(Boolean) + .join("-"); + const traefikKey = sanitize( + [resourceId, traefikPathKey].filter(Boolean).join("-") + ); + + if (!resourcesMap.has(internalMapKey)) { const validation = validatePathRewriteConfig( row.path, row.pathMatchType, @@ -211,9 +228,10 @@ export async function getTraefikConfig( return; } - resourcesMap.set(key, { + resourcesMap.set(internalMapKey, { resourceId: row.resourceId, name: resourceName, + traefikKey: traefikKey, // backward-compatible key for Traefik names fullDomain: row.fullDomain, ssl: row.ssl, http: row.http, @@ -247,7 +265,7 @@ export async function getTraefikConfig( } // Add target with its associated site data - resourcesMap.get(key).targets.push({ + resourcesMap.get(internalMapKey).targets.push({ resourceId: row.resourceId, targetId: row.targetId, ip: row.ip, @@ -300,8 +318,9 @@ export async function getTraefikConfig( }; // get the key and the resource - for (const [key, resource] of resourcesMap.entries()) { + for (const [, resource] of resourcesMap.entries()) { const targets = resource.targets as TargetWithSite[]; + const key = resource.traefikKey; // backward-compatible key for Traefik names const routerName = `${key}-${resource.name}-router`; const serviceName = `${key}-${resource.name}-service`; From 244f497a9c0c0cbdd99ac6ae2336e65c51a3a88b Mon Sep 17 00:00:00 2001 From: Shreyas Date: Mon, 23 Feb 2026 12:33:20 +0530 Subject: [PATCH 3/4] test: add comprehensive backward compatibility tests for path routing fix --- server/lib/traefik/pathEncoding.test.ts | 745 ++++++++++++++++++------ 1 file changed, 574 insertions(+), 171 deletions(-) diff --git a/server/lib/traefik/pathEncoding.test.ts b/server/lib/traefik/pathEncoding.test.ts index a3575b9f2..58dd8653d 100644 --- a/server/lib/traefik/pathEncoding.test.ts +++ b/server/lib/traefik/pathEncoding.test.ts @@ -1,10 +1,93 @@ import { assertEquals } from "../../../test/assert"; import { encodePath, sanitize } from "./pathUtils"; -function runTests() { - console.log("Running path encoding tests...\n"); +// ── Helpers ────────────────────────────────────────────────────────── - // Test 1: null and empty return empty string +/** + * Exact replica of the OLD key computation from upstream main. + * This is what existing Pangolin deployments use today for both + * map grouping AND Traefik router/service names. + * + * Source: origin/main server/lib/traefik/getTraefikConfig.ts lines 130-146 + */ +function oldKeyComputation( + resourceId: number, + path: string | null, + pathMatchType: string | null, + rewritePath: string | null, + rewritePathType: string | null +): string { + const targetPath = sanitize(path) || ""; + const pmt = pathMatchType || ""; + const rp = rewritePath || ""; + const rpt = rewritePathType || ""; + const pathKey = [targetPath, pmt, rp, rpt].filter(Boolean).join("-"); + const mapKey = [resourceId, pathKey].filter(Boolean).join("-"); + return sanitize(mapKey) || ""; +} + +/** + * Replica of the NEW dual-key computation from our fix. + * Returns both the internal map key (for grouping) and the + * Traefik-facing key (for router/service names). + * + * Source: our getTraefikConfig.ts lines 135-163 + */ +function newKeyComputation( + resourceId: number, + path: string | null, + pathMatchType: string | null, + rewritePath: string | null, + rewritePathType: string | null +): { internalMapKey: string; traefikKey: string } { + const pmt = pathMatchType || ""; + const rp = rewritePath || ""; + const rpt = rewritePathType || ""; + + // Internal map key: uses encodePath (collision-free) + const encodedPath = encodePath(path); + const internalPathKey = [encodedPath, pmt, rp, rpt] + .filter(Boolean) + .join("-"); + const internalMapKey = [resourceId, internalPathKey] + .filter(Boolean) + .join("-"); + + // Traefik-facing key: uses sanitize (backward-compatible) + const sanitizedPath = sanitize(path) || ""; + const traefikPathKey = [sanitizedPath, pmt, rp, rpt] + .filter(Boolean) + .join("-"); + const traefikKey = sanitize( + [resourceId, traefikPathKey].filter(Boolean).join("-") + ); + + return { internalMapKey, traefikKey: traefikKey || "" }; +} + +/** + * Build the full Traefik router/service names the way getTraefikConfig does. + */ +function buildTraefikNames(key: string, resourceName: string) { + const name = sanitize(resourceName) || ""; + return { + routerName: `${key}-${name}-router`, + serviceName: `${key}-${name}-service`, + transportName: `${key}-transport`, + headersMiddlewareName: `${key}-headers-middleware` + }; +} + +// ── Tests ──────────────────────────────────────────────────────────── + +function runTests() { + console.log("Running path encoding & backward compatibility tests...\n"); + + let passed = 0; + + // ── encodePath unit tests ──────────────────────────────────────── + + // Test 1: null/undefined/empty { assertEquals(encodePath(null), "", "null should return empty"); assertEquals( @@ -13,131 +96,43 @@ function runTests() { "undefined should return empty" ); assertEquals(encodePath(""), "", "empty string should return empty"); - console.log(" PASS: null/undefined/empty return empty string"); + console.log(" PASS: encodePath handles null/undefined/empty"); + passed++; } - // Test 2: root path "/" encodes to something non-empty + // Test 2: root path { - const result = encodePath("/"); - assertEquals(result !== "", true, "root path should not be empty"); - assertEquals(result, "2f", "root path should encode to hex of '/'"); - console.log(" PASS: root path encodes to non-empty string"); + assertEquals(encodePath("/"), "2f", "/ should encode to 2f"); + console.log(" PASS: encodePath encodes root path"); + passed++; } - // Test 3: different paths produce different encoded values + // Test 3: alphanumeric passthrough { - const paths = [ - "/", - "/api", - "/a/b", - "/a-b", - "/a.b", - "/a_b", - "/api/v1", - "/api/v2" - ]; - const encoded = new Set(); - let collision = false; - for (const p of paths) { - const e = encodePath(p); - if (encoded.has(e)) { - collision = true; - break; - } - encoded.add(e); - } - assertEquals(collision, false, "no two different paths should collide"); - console.log(" PASS: all different paths produce unique encodings"); + assertEquals(encodePath("/api"), "2fapi", "/api encodes slash only"); + assertEquals(encodePath("/v1"), "2fv1", "/v1 encodes slash only"); + assertEquals(encodePath("abc"), "abc", "plain alpha passes through"); + console.log(" PASS: encodePath preserves alphanumeric chars"); + passed++; } - // Test 4: alphanumeric characters pass through unchanged + // Test 4: all special chars produce unique hex { + const paths = ["/a/b", "/a-b", "/a.b", "/a_b", "/a b"]; + const results = paths.map((p) => encodePath(p)); + const unique = new Set(results); assertEquals( - encodePath("/api"), - "2fapi", - "/api should encode slash only" + unique.size, + paths.length, + "all special-char paths must produce unique encodings" ); - assertEquals(encodePath("/v1"), "2fv1", "/v1 should encode slash only"); - console.log(" PASS: alphanumeric characters preserved"); - } - - // Test 5: special characters are hex-encoded - { - const dotEncoded = encodePath("/a.b"); - const dashEncoded = encodePath("/a-b"); - const slashEncoded = encodePath("/a/b"); - const underscoreEncoded = encodePath("/a_b"); - - // all should be different - const set = new Set([ - dotEncoded, - dashEncoded, - slashEncoded, - underscoreEncoded - ]); - assertEquals( - set.size, - 4, - "dot, dash, slash, underscore paths should all be unique" - ); - console.log(" PASS: special characters produce unique encodings"); - } - - // Test 6: full key generation - different paths create different keys - { - function makeKey( - resourceId: number, - path: string | null, - pathMatchType: string | null - ) { - const targetPath = encodePath(path); - const pmt = pathMatchType || ""; - const pathKey = [targetPath, pmt, "", ""].filter(Boolean).join("-"); - const mapKey = [resourceId, pathKey].filter(Boolean).join("-"); - return sanitize(mapKey); - } - - const keySlash = makeKey(1, "/", "prefix"); - const keyApi = makeKey(1, "/api", "prefix"); - const keyNull = makeKey(1, null, null); - - assertEquals( - keySlash !== keyApi, - true, - "/ and /api should have different keys" - ); - assertEquals( - keySlash !== keyNull, - true, - "/ and null should have different keys" - ); - assertEquals( - keyApi !== keyNull, - true, - "/api and null should have different keys" - ); - console.log( - " PASS: different paths create different resource map keys" + " PASS: encodePath produces unique output for different special chars" ); + passed++; } - // Test 7: same path always produces same key (deterministic) - { - assertEquals( - encodePath("/api"), - encodePath("/api"), - "same input should produce same output" - ); - assertEquals( - encodePath("/a/b/c"), - encodePath("/a/b/c"), - "same input should produce same output" - ); - console.log(" PASS: encoding is deterministic"); - } - - // Test 8: encoded result is alphanumeric (valid for Traefik names after sanitize) + // Test 5: output is always alphanumeric (safe for Traefik names) { const paths = [ "/", @@ -149,83 +144,491 @@ function runTests() { ]; for (const p of paths) { const e = encodePath(p); - const isAlphanumeric = /^[a-zA-Z0-9]+$/.test(e); assertEquals( - isAlphanumeric, + /^[a-zA-Z0-9]+$/.test(e), true, - `encodePath("${p}") = "${e}" should be alphanumeric` + `encodePath("${p}") = "${e}" must be alphanumeric` ); } - console.log(" PASS: encoded values are alphanumeric"); + console.log(" PASS: encodePath output is always alphanumeric"); + passed++; } - // Test 9: backward compatibility - Traefik names use sanitize, internal keys use encodePath + // Test 6: deterministic { - // Simulate the dual-key approach from getTraefikConfig - function makeKeys( - resourceId: number, - path: string | null, - pathMatchType: string | null - ) { - // Internal map key (collision-free grouping) - const encodedPath = encodePath(path); - const internalPathKey = [encodedPath, pathMatchType || ""] - .filter(Boolean) - .join("-"); - const internalMapKey = [resourceId, internalPathKey] - .filter(Boolean) - .join("-"); + assertEquals( + encodePath("/api"), + encodePath("/api"), + "same input same output" + ); + assertEquals( + encodePath("/a/b/c"), + encodePath("/a/b/c"), + "same input same output" + ); + console.log(" PASS: encodePath is deterministic"); + passed++; + } - // Traefik-facing key (backward-compatible naming) - const sanitizedPath = sanitize(path) || ""; - const traefikPathKey = [sanitizedPath, pathMatchType || ""] - .filter(Boolean) - .join("-"); - const traefikKey = sanitize( - [resourceId, traefikPathKey].filter(Boolean).join("-") + // Test 7: many distinct paths never collide + { + const paths = [ + "/", + "/api", + "/api/v1", + "/api/v2", + "/a/b", + "/a-b", + "/a.b", + "/a_b", + "/health", + "/health/check", + "/admin", + "/admin/users", + "/api/v1/users", + "/api/v1/posts", + "/app", + "/app/dashboard" + ]; + const encoded = new Set(paths.map((p) => encodePath(p))); + assertEquals( + encoded.size, + paths.length, + `expected ${paths.length} unique encodings, got ${encoded.size}` + ); + console.log(" PASS: 16 realistic paths all produce unique encodings"); + passed++; + } + + // ── Backward compatibility: Traefik names must match old code ───── + + // Test 8: simple resource, no path — Traefik name unchanged + { + const oldKey = oldKeyComputation(1, null, null, null, null); + const { traefikKey } = newKeyComputation(1, null, null, null, null); + assertEquals( + traefikKey, + oldKey, + "no-path resource: Traefik key must match old" + ); + console.log(" PASS: backward compat — no path resource"); + passed++; + } + + // Test 9: resource with /api prefix — Traefik name unchanged + { + const oldKey = oldKeyComputation(1, "/api", "prefix", null, null); + const { traefikKey } = newKeyComputation( + 1, + "/api", + "prefix", + null, + null + ); + assertEquals( + traefikKey, + oldKey, + "/api prefix: Traefik key must match old" + ); + console.log(" PASS: backward compat — /api prefix"); + passed++; + } + + // Test 10: resource with exact path — Traefik name unchanged + { + const oldKey = oldKeyComputation(5, "/health", "exact", null, null); + const { traefikKey } = newKeyComputation( + 5, + "/health", + "exact", + null, + null + ); + assertEquals( + traefikKey, + oldKey, + "/health exact: Traefik key must match old" + ); + console.log(" PASS: backward compat — /health exact"); + passed++; + } + + // Test 11: resource with regex path — Traefik name unchanged + { + const oldKey = oldKeyComputation( + 3, + "^/api/v[0-9]+", + "regex", + null, + null + ); + const { traefikKey } = newKeyComputation( + 3, + "^/api/v[0-9]+", + "regex", + null, + null + ); + assertEquals( + traefikKey, + oldKey, + "regex path: Traefik key must match old" + ); + console.log(" PASS: backward compat — regex path"); + passed++; + } + + // Test 12: resource with path rewrite — Traefik name unchanged + { + const oldKey = oldKeyComputation( + 10, + "/api", + "prefix", + "/backend", + "prefix" + ); + const { traefikKey } = newKeyComputation( + 10, + "/api", + "prefix", + "/backend", + "prefix" + ); + assertEquals( + traefikKey, + oldKey, + "path rewrite: Traefik key must match old" + ); + console.log(" PASS: backward compat — path rewrite (prefix→prefix)"); + passed++; + } + + // Test 13: resource with stripPrefix rewrite — Traefik name unchanged + { + const oldKey = oldKeyComputation( + 7, + "/app", + "prefix", + null, + "stripPrefix" + ); + const { traefikKey } = newKeyComputation( + 7, + "/app", + "prefix", + null, + "stripPrefix" + ); + assertEquals( + traefikKey, + oldKey, + "stripPrefix: Traefik key must match old" + ); + console.log(" PASS: backward compat — stripPrefix rewrite"); + passed++; + } + + // Test 14: root path "/" — Traefik name unchanged + { + const oldKey = oldKeyComputation(1, "/", "prefix", null, null); + const { traefikKey } = newKeyComputation(1, "/", "prefix", null, null); + assertEquals( + traefikKey, + oldKey, + "root path: Traefik key must match old" + ); + console.log(" PASS: backward compat — root path /"); + passed++; + } + + // Test 15: full Traefik router/service names unchanged for existing users + { + const scenarios = [ + { + rid: 1, + name: "my-webapp", + path: "/api", + pmt: "prefix" as const, + rp: null, + rpt: null + }, + { + rid: 2, + name: "backend", + path: "/", + pmt: "prefix" as const, + rp: null, + rpt: null + }, + { + rid: 3, + name: "docs", + path: "/docs", + pmt: "prefix" as const, + rp: "/", + rpt: "stripPrefix" as const + }, + { + rid: 42, + name: "api-service", + path: null, + pmt: null, + rp: null, + rpt: null + }, + { + rid: 100, + name: "grafana", + path: "/grafana", + pmt: "prefix" as const, + rp: null, + rpt: null + } + ]; + for (const s of scenarios) { + const oldKey = oldKeyComputation(s.rid, s.path, s.pmt, s.rp, s.rpt); + const { traefikKey } = newKeyComputation( + s.rid, + s.path, + s.pmt, + s.rp, + s.rpt + ); + const oldNames = buildTraefikNames(oldKey, s.name); + const newNames = buildTraefikNames(traefikKey, s.name); + assertEquals( + newNames.routerName, + oldNames.routerName, + `router name mismatch for resource ${s.rid} ${s.name} path=${s.path}` + ); + assertEquals( + newNames.serviceName, + oldNames.serviceName, + `service name mismatch for resource ${s.rid} ${s.name} path=${s.path}` + ); + assertEquals( + newNames.transportName, + oldNames.transportName, + `transport name mismatch for resource ${s.rid} ${s.name} path=${s.path}` ); - - return { internalMapKey, traefikKey }; } + console.log( + " PASS: backward compat — full router/service/transport names match old code for 5 scenarios" + ); + passed++; + } - // /a/b and /a-b should have DIFFERENT internal keys (no collision) - const keysAB = makeKeys(1, "/a/b", "prefix"); - const keysDash = makeKeys(1, "/a-b", "prefix"); + // Test 16: large resourceId — Traefik name unchanged + { + const oldKey = oldKeyComputation( + 99999, + "/dashboard", + "prefix", + null, + null + ); + const { traefikKey } = newKeyComputation( + 99999, + "/dashboard", + "prefix", + null, + null + ); + assertEquals( + traefikKey, + oldKey, + "large resourceId: Traefik key must match old" + ); + console.log(" PASS: backward compat — large resourceId"); + passed++; + } + + // ── Collision fix: the actual bug we're fixing ─────────────────── + + // Test 17: /a/b and /a-b now have different internal keys (THE BUG FIX) + { + const keysAB = newKeyComputation(1, "/a/b", "prefix", null, null); + const keysDash = newKeyComputation(1, "/a-b", "prefix", null, null); assertEquals( keysAB.internalMapKey !== keysDash.internalMapKey, true, - "/a/b and /a-b must have different internal map keys" + "/a/b and /a-b MUST have different internal map keys" ); - - // /a/b and /a-b produce the SAME Traefik key (backward compat — sanitize maps both to "a-b") - assertEquals( - keysAB.traefikKey, - keysDash.traefikKey, - "/a/b and /a-b should produce same Traefik key (sanitize behavior)" - ); - - // For a resource with no path, Traefik key should match the old behavior - const keysNoPath = makeKeys(42, null, null); - assertEquals( - keysNoPath.traefikKey, - "42", - "no-path resource should have resourceId-only Traefik key" - ); - - // Traefik key for /api prefix should match old sanitize-based output - const keysApi = makeKeys(1, "/api", "prefix"); - assertEquals( - keysApi.traefikKey, - "1-api-prefix", - "Traefik key should match old sanitize-based format" - ); - console.log( - " PASS: backward compatibility - internal keys differ, Traefik names preserved" + " PASS: collision fix — /a/b vs /a-b have different internal keys" ); + passed++; } - console.log("\nAll path encoding tests passed!"); + // Test 18: demonstrate the old bug — old code maps /a/b and /a-b to same key + { + const oldKeyAB = oldKeyComputation(1, "/a/b", "prefix", null, null); + const oldKeyDash = oldKeyComputation(1, "/a-b", "prefix", null, null); + assertEquals( + oldKeyAB, + oldKeyDash, + "old code MUST have this collision (confirms the bug exists)" + ); + console.log(" PASS: confirmed old code bug — /a/b and /a-b collided"); + passed++; + } + + // Test 19: /api/v1 and /api-v1 — old code collision, new code fixes it + { + const oldKey1 = oldKeyComputation(1, "/api/v1", "prefix", null, null); + const oldKey2 = oldKeyComputation(1, "/api-v1", "prefix", null, null); + assertEquals( + oldKey1, + oldKey2, + "old code collision for /api/v1 vs /api-v1" + ); + + const new1 = newKeyComputation(1, "/api/v1", "prefix", null, null); + const new2 = newKeyComputation(1, "/api-v1", "prefix", null, null); + assertEquals( + new1.internalMapKey !== new2.internalMapKey, + true, + "new code must separate /api/v1 and /api-v1" + ); + console.log(" PASS: collision fix — /api/v1 vs /api-v1"); + passed++; + } + + // Test 20: /app.v2 and /app/v2 and /app-v2 — three-way collision fixed + { + const a = newKeyComputation(1, "/app.v2", "prefix", null, null); + const b = newKeyComputation(1, "/app/v2", "prefix", null, null); + const c = newKeyComputation(1, "/app-v2", "prefix", null, null); + const keys = new Set([ + a.internalMapKey, + b.internalMapKey, + c.internalMapKey + ]); + assertEquals( + keys.size, + 3, + "three paths must produce three unique internal keys" + ); + console.log( + " PASS: collision fix — three-way /app.v2, /app/v2, /app-v2" + ); + passed++; + } + + // ── Edge cases ─────────────────────────────────────────────────── + + // Test 21: same path in different resources — always separate + { + const res1 = newKeyComputation(1, "/api", "prefix", null, null); + const res2 = newKeyComputation(2, "/api", "prefix", null, null); + assertEquals( + res1.internalMapKey !== res2.internalMapKey, + true, + "different resources with same path must have different keys" + ); + assertEquals( + res1.traefikKey !== res2.traefikKey, + true, + "different resources with same path must have different Traefik keys" + ); + console.log(" PASS: edge case — same path, different resources"); + passed++; + } + + // Test 22: same resource, different pathMatchType — separate keys + { + const exact = newKeyComputation(1, "/api", "exact", null, null); + const prefix = newKeyComputation(1, "/api", "prefix", null, null); + assertEquals( + exact.internalMapKey !== prefix.internalMapKey, + true, + "exact vs prefix must have different internal keys" + ); + console.log(" PASS: edge case — same path, different match types"); + passed++; + } + + // Test 23: same resource and path, different rewrite config — separate keys + { + const noRewrite = newKeyComputation(1, "/api", "prefix", null, null); + const withRewrite = newKeyComputation( + 1, + "/api", + "prefix", + "/backend", + "prefix" + ); + assertEquals( + noRewrite.internalMapKey !== withRewrite.internalMapKey, + true, + "with vs without rewrite must have different internal keys" + ); + console.log(" PASS: edge case — same path, different rewrite config"); + passed++; + } + + // Test 24: paths with special URL characters + { + const paths = ["/api?foo", "/api#bar", "/api%20baz", "/api+qux"]; + const internal = new Set( + paths.map( + (p) => + newKeyComputation(1, p, "prefix", null, null).internalMapKey + ) + ); + assertEquals( + internal.size, + paths.length, + "special URL chars must produce unique keys" + ); + console.log(" PASS: edge case — special URL characters in paths"); + passed++; + } + + // Test 25: very long path (sanitize truncates at 50 chars — verify consistency) + { + const longPath = "/" + "a".repeat(100); + const oldKey = oldKeyComputation(1, longPath, "prefix", null, null); + const { traefikKey } = newKeyComputation( + 1, + longPath, + "prefix", + null, + null + ); + assertEquals( + traefikKey, + oldKey, + "long path: Traefik key must match old (both truncate)" + ); + console.log(" PASS: edge case — very long path (50-char truncation)"); + passed++; + } + + // Test 26: sticky session cookie safety — service name doesn't change + { + // Sticky sessions use cookie name "p_sticky" tied to the service name. + // If service name changes, existing cookies become invalid. + const oldKey = oldKeyComputation(1, "/api", "prefix", null, null); + const { traefikKey } = newKeyComputation( + 1, + "/api", + "prefix", + null, + null + ); + const oldServiceName = `${oldKey}-my-app-service`; + const newServiceName = `${traefikKey}-my-app-service`; + assertEquals( + newServiceName, + oldServiceName, + "service name must not change (would break sticky session cookies)" + ); + console.log(" PASS: sticky session safety — service name preserved"); + passed++; + } + + console.log(`\nAll ${passed} tests passed!`); } try { From 75a909784af857e27de2de69af74aa4fdd4e80cd Mon Sep 17 00:00:00 2001 From: Shreyas Papinwar Date: Sun, 1 Mar 2026 15:42:24 +0530 Subject: [PATCH 4/4] =?UTF-8?q?fix:=20simplify=20path=20encoding=20per=20r?= =?UTF-8?q?eview=20=E2=80=94=20inline=20utils,=20use=20single=20key=20sche?= =?UTF-8?q?me?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address PR review comments: - Remove pathUtils.ts and move sanitize/encodePath directly into utils.ts - Simplify dual-key approach to single key using encodePath for map keys - Remove backward-compat logic (not needed per reviewer) - Update tests to match simplified approach --- server/lib/traefik/getTraefikConfig.ts | 39 +- server/lib/traefik/pathEncoding.test.ts | 430 +++--------------- server/lib/traefik/pathUtils.ts | 37 -- server/lib/traefik/utils.ts | 34 +- .../private/lib/traefik/getTraefikConfig.ts | 39 +- 5 files changed, 111 insertions(+), 468 deletions(-) delete mode 100644 server/lib/traefik/pathUtils.ts diff --git a/server/lib/traefik/getTraefikConfig.ts b/server/lib/traefik/getTraefikConfig.ts index 3becfb370..7379cad7f 100644 --- a/server/lib/traefik/getTraefikConfig.ts +++ b/server/lib/traefik/getTraefikConfig.ts @@ -127,42 +127,25 @@ export async function getTraefikConfig( resourcesWithTargetsAndSites.forEach((row) => { const resourceId = row.resourceId; const resourceName = sanitize(row.resourceName) || ""; + const targetPath = encodePath(row.path); // Use encodePath to avoid collisions (e.g. "/a/b" vs "/a-b") const pathMatchType = row.pathMatchType || ""; const rewritePath = row.rewritePath || ""; const rewritePathType = row.rewritePathType || ""; const priority = row.priority ?? 100; - // Use encodePath for the internal map key to avoid collisions - // (e.g. "/a/b" vs "/a-b" must map to different groups) - const encodedPath = encodePath(row.path); - const internalPathKey = [ - encodedPath, + // Create a unique key combining resourceId, path config, and rewrite config + const pathKey = [ + targetPath, pathMatchType, rewritePath, rewritePathType ] .filter(Boolean) .join("-"); - const internalMapKey = [resourceId, internalPathKey] - .filter(Boolean) - .join("-"); + const mapKey = [resourceId, pathKey].filter(Boolean).join("-"); + const key = sanitize(mapKey); - // Use sanitize for the Traefik-facing key to preserve backward-compatible - // router/service names (existing sticky session cookies, etc.) - const sanitizedPath = sanitize(row.path) || ""; - const traefikPathKey = [ - sanitizedPath, - pathMatchType, - rewritePath, - rewritePathType - ] - .filter(Boolean) - .join("-"); - const traefikKey = sanitize( - [resourceId, traefikPathKey].filter(Boolean).join("-") - ); - - if (!resourcesMap.has(internalMapKey)) { + if (!resourcesMap.has(mapKey)) { const validation = validatePathRewriteConfig( row.path, row.pathMatchType, @@ -177,10 +160,10 @@ export async function getTraefikConfig( return; } - resourcesMap.set(internalMapKey, { + resourcesMap.set(mapKey, { resourceId: row.resourceId, name: resourceName, - traefikKey: traefikKey, // backward-compatible key for Traefik names + key: key, fullDomain: row.fullDomain, ssl: row.ssl, http: row.http, @@ -208,7 +191,7 @@ export async function getTraefikConfig( }); } - resourcesMap.get(internalMapKey).targets.push({ + resourcesMap.get(mapKey).targets.push({ resourceId: row.resourceId, targetId: row.targetId, ip: row.ip, @@ -247,7 +230,7 @@ export async function getTraefikConfig( // get the key and the resource for (const [, resource] of resourcesMap.entries()) { const targets = resource.targets as TargetWithSite[]; - const key = resource.traefikKey; // backward-compatible key for Traefik names + const key = resource.key; const routerName = `${key}-${resource.name}-router`; const serviceName = `${key}-${resource.name}-service`; diff --git a/server/lib/traefik/pathEncoding.test.ts b/server/lib/traefik/pathEncoding.test.ts index 58dd8653d..83d53a039 100644 --- a/server/lib/traefik/pathEncoding.test.ts +++ b/server/lib/traefik/pathEncoding.test.ts @@ -1,14 +1,30 @@ import { assertEquals } from "../../../test/assert"; -import { encodePath, sanitize } from "./pathUtils"; + +// ── Pure function copies (inlined to avoid pulling in server dependencies) ── + +function sanitize(input: string | null | undefined): string | undefined { + if (!input) return undefined; + if (input.length > 50) { + input = input.substring(0, 50); + } + return input + .replace(/[^a-zA-Z0-9-]/g, "-") + .replace(/-+/g, "-") + .replace(/^-|-$/g, ""); +} + +function encodePath(path: string | null | undefined): string { + if (!path) return ""; + return path.replace(/[^a-zA-Z0-9]/g, (ch) => { + return ch.charCodeAt(0).toString(16); + }); +} // ── Helpers ────────────────────────────────────────────────────────── /** * Exact replica of the OLD key computation from upstream main. - * This is what existing Pangolin deployments use today for both - * map grouping AND Traefik router/service names. - * - * Source: origin/main server/lib/traefik/getTraefikConfig.ts lines 130-146 + * Uses sanitize() for paths — this is what had the collision bug. */ function oldKeyComputation( resourceId: number, @@ -27,11 +43,8 @@ function oldKeyComputation( } /** - * Replica of the NEW dual-key computation from our fix. - * Returns both the internal map key (for grouping) and the - * Traefik-facing key (for router/service names). - * - * Source: our getTraefikConfig.ts lines 135-163 + * Replica of the NEW key computation from our fix. + * Uses encodePath() for paths — collision-free. */ function newKeyComputation( resourceId: number, @@ -39,49 +52,20 @@ function newKeyComputation( pathMatchType: string | null, rewritePath: string | null, rewritePathType: string | null -): { internalMapKey: string; traefikKey: string } { +): string { + const targetPath = encodePath(path); const pmt = pathMatchType || ""; const rp = rewritePath || ""; const rpt = rewritePathType || ""; - - // Internal map key: uses encodePath (collision-free) - const encodedPath = encodePath(path); - const internalPathKey = [encodedPath, pmt, rp, rpt] - .filter(Boolean) - .join("-"); - const internalMapKey = [resourceId, internalPathKey] - .filter(Boolean) - .join("-"); - - // Traefik-facing key: uses sanitize (backward-compatible) - const sanitizedPath = sanitize(path) || ""; - const traefikPathKey = [sanitizedPath, pmt, rp, rpt] - .filter(Boolean) - .join("-"); - const traefikKey = sanitize( - [resourceId, traefikPathKey].filter(Boolean).join("-") - ); - - return { internalMapKey, traefikKey: traefikKey || "" }; -} - -/** - * Build the full Traefik router/service names the way getTraefikConfig does. - */ -function buildTraefikNames(key: string, resourceName: string) { - const name = sanitize(resourceName) || ""; - return { - routerName: `${key}-${name}-router`, - serviceName: `${key}-${name}-service`, - transportName: `${key}-transport`, - headersMiddlewareName: `${key}-headers-middleware` - }; + const pathKey = [targetPath, pmt, rp, rpt].filter(Boolean).join("-"); + const mapKey = [resourceId, pathKey].filter(Boolean).join("-"); + return sanitize(mapKey) || ""; } // ── Tests ──────────────────────────────────────────────────────────── function runTests() { - console.log("Running path encoding & backward compatibility tests...\n"); + console.log("Running path encoding tests...\n"); let passed = 0; @@ -200,267 +184,22 @@ function runTests() { passed++; } - // ── Backward compatibility: Traefik names must match old code ───── - - // Test 8: simple resource, no path — Traefik name unchanged - { - const oldKey = oldKeyComputation(1, null, null, null, null); - const { traefikKey } = newKeyComputation(1, null, null, null, null); - assertEquals( - traefikKey, - oldKey, - "no-path resource: Traefik key must match old" - ); - console.log(" PASS: backward compat — no path resource"); - passed++; - } - - // Test 9: resource with /api prefix — Traefik name unchanged - { - const oldKey = oldKeyComputation(1, "/api", "prefix", null, null); - const { traefikKey } = newKeyComputation( - 1, - "/api", - "prefix", - null, - null - ); - assertEquals( - traefikKey, - oldKey, - "/api prefix: Traefik key must match old" - ); - console.log(" PASS: backward compat — /api prefix"); - passed++; - } - - // Test 10: resource with exact path — Traefik name unchanged - { - const oldKey = oldKeyComputation(5, "/health", "exact", null, null); - const { traefikKey } = newKeyComputation( - 5, - "/health", - "exact", - null, - null - ); - assertEquals( - traefikKey, - oldKey, - "/health exact: Traefik key must match old" - ); - console.log(" PASS: backward compat — /health exact"); - passed++; - } - - // Test 11: resource with regex path — Traefik name unchanged - { - const oldKey = oldKeyComputation( - 3, - "^/api/v[0-9]+", - "regex", - null, - null - ); - const { traefikKey } = newKeyComputation( - 3, - "^/api/v[0-9]+", - "regex", - null, - null - ); - assertEquals( - traefikKey, - oldKey, - "regex path: Traefik key must match old" - ); - console.log(" PASS: backward compat — regex path"); - passed++; - } - - // Test 12: resource with path rewrite — Traefik name unchanged - { - const oldKey = oldKeyComputation( - 10, - "/api", - "prefix", - "/backend", - "prefix" - ); - const { traefikKey } = newKeyComputation( - 10, - "/api", - "prefix", - "/backend", - "prefix" - ); - assertEquals( - traefikKey, - oldKey, - "path rewrite: Traefik key must match old" - ); - console.log(" PASS: backward compat — path rewrite (prefix→prefix)"); - passed++; - } - - // Test 13: resource with stripPrefix rewrite — Traefik name unchanged - { - const oldKey = oldKeyComputation( - 7, - "/app", - "prefix", - null, - "stripPrefix" - ); - const { traefikKey } = newKeyComputation( - 7, - "/app", - "prefix", - null, - "stripPrefix" - ); - assertEquals( - traefikKey, - oldKey, - "stripPrefix: Traefik key must match old" - ); - console.log(" PASS: backward compat — stripPrefix rewrite"); - passed++; - } - - // Test 14: root path "/" — Traefik name unchanged - { - const oldKey = oldKeyComputation(1, "/", "prefix", null, null); - const { traefikKey } = newKeyComputation(1, "/", "prefix", null, null); - assertEquals( - traefikKey, - oldKey, - "root path: Traefik key must match old" - ); - console.log(" PASS: backward compat — root path /"); - passed++; - } - - // Test 15: full Traefik router/service names unchanged for existing users - { - const scenarios = [ - { - rid: 1, - name: "my-webapp", - path: "/api", - pmt: "prefix" as const, - rp: null, - rpt: null - }, - { - rid: 2, - name: "backend", - path: "/", - pmt: "prefix" as const, - rp: null, - rpt: null - }, - { - rid: 3, - name: "docs", - path: "/docs", - pmt: "prefix" as const, - rp: "/", - rpt: "stripPrefix" as const - }, - { - rid: 42, - name: "api-service", - path: null, - pmt: null, - rp: null, - rpt: null - }, - { - rid: 100, - name: "grafana", - path: "/grafana", - pmt: "prefix" as const, - rp: null, - rpt: null - } - ]; - for (const s of scenarios) { - const oldKey = oldKeyComputation(s.rid, s.path, s.pmt, s.rp, s.rpt); - const { traefikKey } = newKeyComputation( - s.rid, - s.path, - s.pmt, - s.rp, - s.rpt - ); - const oldNames = buildTraefikNames(oldKey, s.name); - const newNames = buildTraefikNames(traefikKey, s.name); - assertEquals( - newNames.routerName, - oldNames.routerName, - `router name mismatch for resource ${s.rid} ${s.name} path=${s.path}` - ); - assertEquals( - newNames.serviceName, - oldNames.serviceName, - `service name mismatch for resource ${s.rid} ${s.name} path=${s.path}` - ); - assertEquals( - newNames.transportName, - oldNames.transportName, - `transport name mismatch for resource ${s.rid} ${s.name} path=${s.path}` - ); - } - console.log( - " PASS: backward compat — full router/service/transport names match old code for 5 scenarios" - ); - passed++; - } - - // Test 16: large resourceId — Traefik name unchanged - { - const oldKey = oldKeyComputation( - 99999, - "/dashboard", - "prefix", - null, - null - ); - const { traefikKey } = newKeyComputation( - 99999, - "/dashboard", - "prefix", - null, - null - ); - assertEquals( - traefikKey, - oldKey, - "large resourceId: Traefik key must match old" - ); - console.log(" PASS: backward compat — large resourceId"); - passed++; - } - // ── Collision fix: the actual bug we're fixing ─────────────────── - // Test 17: /a/b and /a-b now have different internal keys (THE BUG FIX) + // Test 8: /a/b and /a-b now have different keys (THE BUG FIX) { - const keysAB = newKeyComputation(1, "/a/b", "prefix", null, null); - const keysDash = newKeyComputation(1, "/a-b", "prefix", null, null); + const keyAB = newKeyComputation(1, "/a/b", "prefix", null, null); + const keyDash = newKeyComputation(1, "/a-b", "prefix", null, null); assertEquals( - keysAB.internalMapKey !== keysDash.internalMapKey, + keyAB !== keyDash, true, - "/a/b and /a-b MUST have different internal map keys" - ); - console.log( - " PASS: collision fix — /a/b vs /a-b have different internal keys" + "/a/b and /a-b MUST have different keys" ); + console.log(" PASS: collision fix — /a/b vs /a-b have different keys"); passed++; } - // Test 18: demonstrate the old bug — old code maps /a/b and /a-b to same key + // Test 9: demonstrate the old bug — old code maps /a/b and /a-b to same key { const oldKeyAB = oldKeyComputation(1, "/a/b", "prefix", null, null); const oldKeyDash = oldKeyComputation(1, "/a-b", "prefix", null, null); @@ -473,7 +212,7 @@ function runTests() { passed++; } - // Test 19: /api/v1 and /api-v1 — old code collision, new code fixes it + // Test 10: /api/v1 and /api-v1 — old code collision, new code fixes it { const oldKey1 = oldKeyComputation(1, "/api/v1", "prefix", null, null); const oldKey2 = oldKeyComputation(1, "/api-v1", "prefix", null, null); @@ -483,10 +222,10 @@ function runTests() { "old code collision for /api/v1 vs /api-v1" ); - const new1 = newKeyComputation(1, "/api/v1", "prefix", null, null); - const new2 = newKeyComputation(1, "/api-v1", "prefix", null, null); + const newKey1 = newKeyComputation(1, "/api/v1", "prefix", null, null); + const newKey2 = newKeyComputation(1, "/api-v1", "prefix", null, null); assertEquals( - new1.internalMapKey !== new2.internalMapKey, + newKey1 !== newKey2, true, "new code must separate /api/v1 and /api-v1" ); @@ -494,20 +233,16 @@ function runTests() { passed++; } - // Test 20: /app.v2 and /app/v2 and /app-v2 — three-way collision fixed + // Test 11: /app.v2 and /app/v2 and /app-v2 — three-way collision fixed { const a = newKeyComputation(1, "/app.v2", "prefix", null, null); const b = newKeyComputation(1, "/app/v2", "prefix", null, null); const c = newKeyComputation(1, "/app-v2", "prefix", null, null); - const keys = new Set([ - a.internalMapKey, - b.internalMapKey, - c.internalMapKey - ]); + const keys = new Set([a, b, c]); assertEquals( keys.size, 3, - "three paths must produce three unique internal keys" + "three paths must produce three unique keys" ); console.log( " PASS: collision fix — three-way /app.v2, /app/v2, /app-v2" @@ -517,38 +252,33 @@ function runTests() { // ── Edge cases ─────────────────────────────────────────────────── - // Test 21: same path in different resources — always separate + // Test 12: same path in different resources — always separate { - const res1 = newKeyComputation(1, "/api", "prefix", null, null); - const res2 = newKeyComputation(2, "/api", "prefix", null, null); + const key1 = newKeyComputation(1, "/api", "prefix", null, null); + const key2 = newKeyComputation(2, "/api", "prefix", null, null); assertEquals( - res1.internalMapKey !== res2.internalMapKey, + key1 !== key2, true, "different resources with same path must have different keys" ); - assertEquals( - res1.traefikKey !== res2.traefikKey, - true, - "different resources with same path must have different Traefik keys" - ); console.log(" PASS: edge case — same path, different resources"); passed++; } - // Test 22: same resource, different pathMatchType — separate keys + // Test 13: same resource, different pathMatchType — separate keys { const exact = newKeyComputation(1, "/api", "exact", null, null); const prefix = newKeyComputation(1, "/api", "prefix", null, null); assertEquals( - exact.internalMapKey !== prefix.internalMapKey, + exact !== prefix, true, - "exact vs prefix must have different internal keys" + "exact vs prefix must have different keys" ); console.log(" PASS: edge case — same path, different match types"); passed++; } - // Test 23: same resource and path, different rewrite config — separate keys + // Test 14: same resource and path, different rewrite config — separate keys { const noRewrite = newKeyComputation(1, "/api", "prefix", null, null); const withRewrite = newKeyComputation( @@ -559,25 +289,22 @@ function runTests() { "prefix" ); assertEquals( - noRewrite.internalMapKey !== withRewrite.internalMapKey, + noRewrite !== withRewrite, true, - "with vs without rewrite must have different internal keys" + "with vs without rewrite must have different keys" ); console.log(" PASS: edge case — same path, different rewrite config"); passed++; } - // Test 24: paths with special URL characters + // Test 15: paths with special URL characters { const paths = ["/api?foo", "/api#bar", "/api%20baz", "/api+qux"]; - const internal = new Set( - paths.map( - (p) => - newKeyComputation(1, p, "prefix", null, null).internalMapKey - ) + const keys = new Set( + paths.map((p) => newKeyComputation(1, p, "prefix", null, null)) ); assertEquals( - internal.size, + keys.size, paths.length, "special URL chars must produce unique keys" ); @@ -585,49 +312,6 @@ function runTests() { passed++; } - // Test 25: very long path (sanitize truncates at 50 chars — verify consistency) - { - const longPath = "/" + "a".repeat(100); - const oldKey = oldKeyComputation(1, longPath, "prefix", null, null); - const { traefikKey } = newKeyComputation( - 1, - longPath, - "prefix", - null, - null - ); - assertEquals( - traefikKey, - oldKey, - "long path: Traefik key must match old (both truncate)" - ); - console.log(" PASS: edge case — very long path (50-char truncation)"); - passed++; - } - - // Test 26: sticky session cookie safety — service name doesn't change - { - // Sticky sessions use cookie name "p_sticky" tied to the service name. - // If service name changes, existing cookies become invalid. - const oldKey = oldKeyComputation(1, "/api", "prefix", null, null); - const { traefikKey } = newKeyComputation( - 1, - "/api", - "prefix", - null, - null - ); - const oldServiceName = `${oldKey}-my-app-service`; - const newServiceName = `${traefikKey}-my-app-service`; - assertEquals( - newServiceName, - oldServiceName, - "service name must not change (would break sticky session cookies)" - ); - console.log(" PASS: sticky session safety — service name preserved"); - passed++; - } - console.log(`\nAll ${passed} tests passed!`); } diff --git a/server/lib/traefik/pathUtils.ts b/server/lib/traefik/pathUtils.ts deleted file mode 100644 index 1b9d57a89..000000000 --- a/server/lib/traefik/pathUtils.ts +++ /dev/null @@ -1,37 +0,0 @@ -/** - * Pure utility functions for path/name encoding. - * No external dependencies — safe to import in tests. - */ - -export function sanitize(input: string | null | undefined): string | undefined { - if (!input) return undefined; - // clean any non alphanumeric characters from the input and replace with dashes - // the input cant be too long either, so limit to 50 characters - if (input.length > 50) { - input = input.substring(0, 50); - } - return input - .replace(/[^a-zA-Z0-9-]/g, "-") - .replace(/-+/g, "-") - .replace(/^-|-$/g, ""); -} - -/** - * Encode a URL path into a collision-free alphanumeric string suitable for use - * in Traefik map keys. - * - * Unlike sanitize(), this preserves uniqueness by encoding each non-alphanumeric - * character as its hex code. Different paths always produce different outputs. - * - * encodePath("/api") => "2fapi" - * encodePath("/a/b") => "2fa2fb" - * encodePath("/a-b") => "2fa2db" (different from /a/b) - * encodePath("/") => "2f" - * encodePath(null) => "" - */ -export function encodePath(path: string | null | undefined): string { - if (!path) return ""; - return path.replace(/[^a-zA-Z0-9]/g, (ch) => { - return ch.charCodeAt(0).toString(16); - }); -} diff --git a/server/lib/traefik/utils.ts b/server/lib/traefik/utils.ts index f40e2eba2..34c293340 100644 --- a/server/lib/traefik/utils.ts +++ b/server/lib/traefik/utils.ts @@ -1,7 +1,37 @@ import logger from "@server/logger"; -// Re-export pure functions from dependency-free module -export { sanitize, encodePath } from "./pathUtils"; +export function sanitize(input: string | null | undefined): string | undefined { + if (!input) return undefined; + // clean any non alphanumeric characters from the input and replace with dashes + // the input cant be too long either, so limit to 50 characters + if (input.length > 50) { + input = input.substring(0, 50); + } + return input + .replace(/[^a-zA-Z0-9-]/g, "-") + .replace(/-+/g, "-") + .replace(/^-|-$/g, ""); +} + +/** + * Encode a URL path into a collision-free alphanumeric string suitable for use + * in Traefik map keys. + * + * Unlike sanitize(), this preserves uniqueness by encoding each non-alphanumeric + * character as its hex code. Different paths always produce different outputs. + * + * encodePath("/api") => "2fapi" + * encodePath("/a/b") => "2fa2fb" + * encodePath("/a-b") => "2fa2db" (different from /a/b) + * encodePath("/") => "2f" + * encodePath(null) => "" + */ +export function encodePath(path: string | null | undefined): string { + if (!path) return ""; + return path.replace(/[^a-zA-Z0-9]/g, (ch) => { + return ch.charCodeAt(0).toString(16); + }); +} export function validatePathRewriteConfig( path: string | null, diff --git a/server/private/lib/traefik/getTraefikConfig.ts b/server/private/lib/traefik/getTraefikConfig.ts index b6c460072..adc3d965b 100644 --- a/server/private/lib/traefik/getTraefikConfig.ts +++ b/server/private/lib/traefik/getTraefikConfig.ts @@ -174,6 +174,7 @@ export async function getTraefikConfig( resourcesWithTargetsAndSites.forEach((row) => { const resourceId = row.resourceId; const resourceName = sanitize(row.resourceName) || ""; + const targetPath = encodePath(row.path); // Use encodePath to avoid collisions (e.g. "/a/b" vs "/a-b") const pathMatchType = row.pathMatchType || ""; const rewritePath = row.rewritePath || ""; const rewritePathType = row.rewritePathType || ""; @@ -183,37 +184,19 @@ export async function getTraefikConfig( return; } - // Use encodePath for the internal map key to avoid collisions - // (e.g. "/a/b" vs "/a-b" must map to different groups) - const encodedPath = encodePath(row.path); - const internalPathKey = [ - encodedPath, + // Create a unique key combining resourceId, path config, and rewrite config + const pathKey = [ + targetPath, pathMatchType, rewritePath, rewritePathType ] .filter(Boolean) .join("-"); - const internalMapKey = [resourceId, internalPathKey] - .filter(Boolean) - .join("-"); + const mapKey = [resourceId, pathKey].filter(Boolean).join("-"); + const key = sanitize(mapKey); - // Use sanitize for the Traefik-facing key to preserve backward-compatible - // router/service names (existing sticky session cookies, etc.) - const sanitizedPath = sanitize(row.path) || ""; - const traefikPathKey = [ - sanitizedPath, - pathMatchType, - rewritePath, - rewritePathType - ] - .filter(Boolean) - .join("-"); - const traefikKey = sanitize( - [resourceId, traefikPathKey].filter(Boolean).join("-") - ); - - if (!resourcesMap.has(internalMapKey)) { + if (!resourcesMap.has(mapKey)) { const validation = validatePathRewriteConfig( row.path, row.pathMatchType, @@ -228,10 +211,10 @@ export async function getTraefikConfig( return; } - resourcesMap.set(internalMapKey, { + resourcesMap.set(mapKey, { resourceId: row.resourceId, name: resourceName, - traefikKey: traefikKey, // backward-compatible key for Traefik names + key: key, fullDomain: row.fullDomain, ssl: row.ssl, http: row.http, @@ -265,7 +248,7 @@ export async function getTraefikConfig( } // Add target with its associated site data - resourcesMap.get(internalMapKey).targets.push({ + resourcesMap.get(mapKey).targets.push({ resourceId: row.resourceId, targetId: row.targetId, ip: row.ip, @@ -320,7 +303,7 @@ export async function getTraefikConfig( // get the key and the resource for (const [, resource] of resourcesMap.entries()) { const targets = resource.targets as TargetWithSite[]; - const key = resource.traefikKey; // backward-compatible key for Traefik names + const key = resource.key; const routerName = `${key}-${resource.name}-router`; const serviceName = `${key}-${resource.name}-service`;