Merge pull request #2524 from shreyaspapi/fix/2294-path-based-routing

fix: path-based routing broken due to key collisions in sanitize()

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<any> {
     // 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); // Use encodePath to avoid collisions (e.g. "/a/b" vs "/a-b")
         const pathMatchType = row.pathMatchType || "";
         const rewritePath = row.rewritePath || "";
         const rewritePathType = row.rewritePathType || "";
@@ -145,7 +145,7 @@ export async function getTraefikConfig(
         const mapKey = [resourceId, pathKey].filter(Boolean).join("-");
         const key = sanitize(mapKey);
 
-        if (!resourcesMap.has(key)) {
+        if (!resourcesMap.has(mapKey)) {
             const validation = validatePathRewriteConfig(
                 row.path,
                 row.pathMatchType,
@@ -160,9 +160,10 @@ export async function getTraefikConfig(
                 return;
             }
 
-            resourcesMap.set(key, {
+            resourcesMap.set(mapKey, {
                 resourceId: row.resourceId,
                 name: resourceName,
+                key: key,
                 fullDomain: row.fullDomain,
                 ssl: row.ssl,
                 http: row.http,
@@ -190,7 +191,7 @@ export async function getTraefikConfig(
             });
         }
 
-        resourcesMap.get(key).targets.push({
+        resourcesMap.get(mapKey).targets.push({
             resourceId: row.resourceId,
             targetId: row.targetId,
             ip: row.ip,
@@ -227,8 +228,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.key;
 
         const routerName = `${key}-${resource.name}-router`;
         const serviceName = `${key}-${resource.name}-service`;

server/lib/traefik/pathEncoding.test.ts

@@ -0,0 +1,323 @@
+import { assertEquals } from "../../../test/assert";
+
+// ── 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.
+ * Uses sanitize() for paths — this is what had the collision bug.
+ */
+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 key computation from our fix.
+ * Uses encodePath() for paths — collision-free.
+ */
+function newKeyComputation(
+    resourceId: number,
+    path: string | null,
+    pathMatchType: string | null,
+    rewritePath: string | null,
+    rewritePathType: string | null
+): string {
+    const targetPath = encodePath(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) || "";
+}
+
+// ── Tests ────────────────────────────────────────────────────────────
+
+function runTests() {
+    console.log("Running path encoding tests...\n");
+
+    let passed = 0;
+
+    // ── encodePath unit tests ────────────────────────────────────────
+
+    // Test 1: null/undefined/empty
+    {
+        assertEquals(encodePath(null), "", "null should return empty");
+        assertEquals(
+            encodePath(undefined),
+            "",
+            "undefined should return empty"
+        );
+        assertEquals(encodePath(""), "", "empty string should return empty");
+        console.log("  PASS: encodePath handles null/undefined/empty");
+        passed++;
+    }
+
+    // Test 2: root path
+    {
+        assertEquals(encodePath("/"), "2f", "/ should encode to 2f");
+        console.log("  PASS: encodePath encodes root path");
+        passed++;
+    }
+
+    // Test 3: alphanumeric passthrough
+    {
+        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: 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(
+            unique.size,
+            paths.length,
+            "all special-char paths must produce unique encodings"
+        );
+        console.log(
+            "  PASS: encodePath produces unique output for different special chars"
+        );
+        passed++;
+    }
+
+    // Test 5: output is always alphanumeric (safe for Traefik names)
+    {
+        const paths = [
+            "/",
+            "/api",
+            "/a/b",
+            "/a-b",
+            "/a.b",
+            "/complex/path/here"
+        ];
+        for (const p of paths) {
+            const e = encodePath(p);
+            assertEquals(
+                /^[a-zA-Z0-9]+$/.test(e),
+                true,
+                `encodePath("${p}") = "${e}" must be alphanumeric`
+            );
+        }
+        console.log("  PASS: encodePath output is always alphanumeric");
+        passed++;
+    }
+
+    // Test 6: deterministic
+    {
+        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++;
+    }
+
+    // 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++;
+    }
+
+    // ── Collision fix: the actual bug we're fixing ───────────────────
+
+    // Test 8: /a/b and /a-b now have different keys (THE BUG FIX)
+    {
+        const keyAB = newKeyComputation(1, "/a/b", "prefix", null, null);
+        const keyDash = newKeyComputation(1, "/a-b", "prefix", null, null);
+        assertEquals(
+            keyAB !== keyDash,
+            true,
+            "/a/b and /a-b MUST have different keys"
+        );
+        console.log("  PASS: collision fix — /a/b vs /a-b have different keys");
+        passed++;
+    }
+
+    // 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);
+        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 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);
+        assertEquals(
+            oldKey1,
+            oldKey2,
+            "old code collision for /api/v1 vs /api-v1"
+        );
+
+        const newKey1 = newKeyComputation(1, "/api/v1", "prefix", null, null);
+        const newKey2 = newKeyComputation(1, "/api-v1", "prefix", null, null);
+        assertEquals(
+            newKey1 !== newKey2,
+            true,
+            "new code must separate /api/v1 and /api-v1"
+        );
+        console.log("  PASS: collision fix — /api/v1 vs /api-v1");
+        passed++;
+    }
+
+    // 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, b, c]);
+        assertEquals(
+            keys.size,
+            3,
+            "three paths must produce three unique keys"
+        );
+        console.log(
+            "  PASS: collision fix — three-way /app.v2, /app/v2, /app-v2"
+        );
+        passed++;
+    }
+
+    // ── Edge cases ───────────────────────────────────────────────────
+
+    // Test 12: same path in different resources — always separate
+    {
+        const key1 = newKeyComputation(1, "/api", "prefix", null, null);
+        const key2 = newKeyComputation(2, "/api", "prefix", null, null);
+        assertEquals(
+            key1 !== key2,
+            true,
+            "different resources with same path must have different keys"
+        );
+        console.log("  PASS: edge case — same path, different resources");
+        passed++;
+    }
+
+    // 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 !== prefix,
+            true,
+            "exact vs prefix must have different keys"
+        );
+        console.log("  PASS: edge case — same path, different match types");
+        passed++;
+    }
+
+    // Test 14: 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 !== withRewrite,
+            true,
+            "with vs without rewrite must have different keys"
+        );
+        console.log("  PASS: edge case — same path, different rewrite config");
+        passed++;
+    }
+
+    // Test 15: paths with special URL characters
+    {
+        const paths = ["/api?foo", "/api#bar", "/api%20baz", "/api+qux"];
+        const keys = new Set(
+            paths.map((p) => newKeyComputation(1, p, "prefix", null, null))
+        );
+        assertEquals(
+            keys.size,
+            paths.length,
+            "special URL chars must produce unique keys"
+        );
+        console.log("  PASS: edge case — special URL characters in paths");
+        passed++;
+    }
+
+    console.log(`\nAll ${passed} tests passed!`);
+}
+
+try {
+    runTests();
+} catch (error) {
+    console.error("Test failed:", error);
+    process.exit(1);
+}

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 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,

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); // Use encodePath to avoid collisions (e.g. "/a/b" vs "/a-b")
         const pathMatchType = row.pathMatchType || "";
         const rewritePath = row.rewritePath || "";
         const rewritePathType = row.rewritePathType || "";
@@ -192,7 +196,7 @@ export async function getTraefikConfig(
         const mapKey = [resourceId, pathKey].filter(Boolean).join("-");
         const key = sanitize(mapKey);
 
-        if (!resourcesMap.has(key)) {
+        if (!resourcesMap.has(mapKey)) {
             const validation = validatePathRewriteConfig(
                 row.path,
                 row.pathMatchType,
@@ -207,9 +211,10 @@ export async function getTraefikConfig(
                 return;
             }
 
-            resourcesMap.set(key, {
+            resourcesMap.set(mapKey, {
                 resourceId: row.resourceId,
                 name: resourceName,
+                key: key,
                 fullDomain: row.fullDomain,
                 ssl: row.ssl,
                 http: row.http,
@@ -243,7 +248,7 @@ export async function getTraefikConfig(
         }
 
         // Add target with its associated site data
-        resourcesMap.get(key).targets.push({
+        resourcesMap.get(mapKey).targets.push({
             resourceId: row.resourceId,
             targetId: row.targetId,
             ip: row.ip,
@@ -296,8 +301,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.key;
 
         const routerName = `${key}-${resource.name}-router`;
         const serviceName = `${key}-${resource.name}-service`;