From e26d7a2c3f148df4b2b042ff87b7e717661024a3 Mon Sep 17 00:00:00 2001
From: fuomag9 <1580624+fuomag9@users.noreply.github.com>
Date: Sat, 28 Mar 2026 14:49:56 +0100
Subject: [PATCH] feat: improve LocationRulesFields UI and add unit tests for
 buildLocationReverseProxy

- Replace textarea with per-upstream rows (protocol dropdown + address input),
  matching the existing UpstreamInput component pattern
- Export buildLocationReverseProxy for testing
- Add 14 unit tests covering: dial formatting, HTTPS/TLS transport,
  host header preservation, path sanitization, IPv6, mixed upstreams

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
---
 .../proxy-hosts/LocationRulesFields.tsx       | 182 ++++++++++++-----
 src/lib/caddy.ts                              |   8 +-
 tests/unit/caddy-location-rules.test.ts       | 184 ++++++++++++++++++
 3 files changed, 327 insertions(+), 47 deletions(-)
 create mode 100644 tests/unit/caddy-location-rules.test.ts

diff --git a/src/components/proxy-hosts/LocationRulesFields.tsx b/src/components/proxy-hosts/LocationRulesFields.tsx
index f03ab62a..c7db0454 100644
--- a/src/components/proxy-hosts/LocationRulesFields.tsx
+++ b/src/components/proxy-hosts/LocationRulesFields.tsx
@@ -2,17 +2,52 @@
 import { useState } from "react";
 import { Button } from "@/components/ui/button";
 import { Input } from "@/components/ui/input";
-import { Textarea } from "@/components/ui/textarea";
-import { Trash2, Plus } from "lucide-react";
+import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from "@/components/ui/select";
+import { Trash2, Plus, MinusCircle } from "lucide-react";
 import type { LocationRule } from "@/lib/models/proxy-hosts";
 
+type UpstreamEntry = { protocol: string; address: string };
+
+function parseUpstream(upstream: string): UpstreamEntry {
+  if (upstream.startsWith("https://")) return { protocol: "https://", address: upstream.slice(8) };
+  if (upstream.startsWith("http://")) return { protocol: "http://", address: upstream.slice(7) };
+  return { protocol: "http://", address: upstream };
+}
+
+function serializeUpstream(entry: UpstreamEntry): string {
+  return `${entry.protocol}${entry.address.trim()}`;
+}
+
+type RuleState = { path: string; upstreams: UpstreamEntry[] };
+
+function toState(rules: LocationRule[]): RuleState[] {
+  return rules.map((r) => ({
+    path: r.path,
+    upstreams: r.upstreams.length > 0 ? r.upstreams.map(parseUpstream) : [{ protocol: "http://", address: "" }],
+  }));
+}
+
+function toJson(rules: RuleState[]): string {
+  return JSON.stringify(
+    rules
+      .filter((r) => r.path.trim())
+      .map((r) => ({
+        path: r.path.trim(),
+        upstreams: r.upstreams
+          .filter((u) => u.address.trim())
+          .map(serializeUpstream),
+      }))
+      .filter((r) => r.upstreams.length > 0)
+  );
+}
+
 type Props = { initialData?: LocationRule[] };
 
 export function LocationRulesFields({ initialData = [] }: Props) {
-  const [rules, setRules] = useState<LocationRule[]>(initialData);
+  const [rules, setRules] = useState<RuleState[]>(toState(initialData));
 
   const addRule = () =>
-    setRules((r) => [...r, { path: "", upstreams: [] }]);
+    setRules((r) => [...r, { path: "", upstreams: [{ protocol: "http://", address: "" }] }]);
 
   const removeRule = (i: number) =>
     setRules((r) => r.filter((_, idx) => idx !== i));
@@ -20,63 +55,122 @@ export function LocationRulesFields({ initialData = [] }: Props) {
   const updatePath = (i: number, value: string) =>
     setRules((r) => r.map((rule, idx) => (idx === i ? { ...rule, path: value } : rule)));
 
-  const updateUpstreams = (i: number, value: string) =>
+  const addUpstream = (ruleIdx: number) =>
     setRules((r) =>
       r.map((rule, idx) =>
-        idx === i
+        idx === ruleIdx
+          ? { ...rule, upstreams: [...rule.upstreams, { protocol: "http://", address: "" }] }
+          : rule
+      )
+    );
+
+  const removeUpstream = (ruleIdx: number, upIdx: number) =>
+    setRules((r) =>
+      r.map((rule, idx) =>
+        idx === ruleIdx && rule.upstreams.length > 1
+          ? { ...rule, upstreams: rule.upstreams.filter((_, i) => i !== upIdx) }
+          : rule
+      )
+    );
+
+  const updateUpstreamProtocol = (ruleIdx: number, upIdx: number, protocol: string) =>
+    setRules((r) =>
+      r.map((rule, idx) =>
+        idx === ruleIdx
           ? {
               ...rule,
-              upstreams: value
-                .split("\n")
-                .map((u) => u.trim())
-                .filter(Boolean),
+              upstreams: rule.upstreams.map((u, i) => (i === upIdx ? { ...u, protocol } : u)),
             }
           : rule
       )
     );
 
+  const updateUpstreamAddress = (ruleIdx: number, upIdx: number, address: string) =>
+    setRules((r) =>
+      r.map((rule, idx) => {
+        if (idx !== ruleIdx) return rule;
+        return {
+          ...rule,
+          upstreams: rule.upstreams.map((u, i) => {
+            if (i !== upIdx) return u;
+            if (address.startsWith("https://")) return { protocol: "https://", address: address.slice(8) };
+            if (address.startsWith("http://")) return { protocol: "http://", address: address.slice(7) };
+            return { ...u, address };
+          }),
+        };
+      })
+    );
+
   return (
     <div>
       <p className="text-sm font-semibold mb-2">Location Rules</p>
-      <input type="hidden" name="location_rules_json" value={JSON.stringify(rules)} />
+      <input type="hidden" name="location_rules_json" value={toJson(rules)} />
       {rules.length > 0 && (
-        <div className="mb-2 flex flex-col gap-3">
+        <div className="mb-2 flex flex-col gap-4">
           {rules.map((rule, i) => (
-            <div key={i} className="grid grid-cols-[1fr_1fr_40px] gap-2 items-start">
-              <div>
-                {i === 0 && (
+            <div key={i} className="rounded-md border p-3 flex flex-col gap-2">
+              <div className="flex items-center gap-2">
+                <div className="flex-1">
                   <span className="text-xs font-medium text-muted-foreground px-1 mb-1 block">Path Pattern</span>
-                )}
-                <Input
-                  size={1}
-                  placeholder="/ws/*"
-                  value={rule.path}
-                  onChange={(e) => updatePath(i, e.target.value)}
-                  className="h-8 text-sm"
-                />
+                  <Input
+                    size={1}
+                    placeholder="/ws/*"
+                    value={rule.path}
+                    onChange={(e) => updatePath(i, e.target.value)}
+                    className="h-8 text-sm"
+                  />
+                </div>
+                <div className="self-end">
+                  <Button
+                    type="button"
+                    variant="ghost"
+                    size="icon"
+                    className="h-8 w-8 text-destructive hover:text-destructive"
+                    onClick={() => removeRule(i)}
+                  >
+                    <Trash2 className="h-4 w-4" />
+                  </Button>
+                </div>
               </div>
               <div>
-                {i === 0 && (
-                  <span className="text-xs font-medium text-muted-foreground px-1 mb-1 block">Upstreams</span>
-                )}
-                <Textarea
-                  placeholder={"ws-backend:8080\nws-backend2:8080"}
-                  value={rule.upstreams.join("\n")}
-                  onChange={(e) => updateUpstreams(i, e.target.value)}
-                  className="text-sm min-h-[32px]"
-                  rows={Math.max(1, rule.upstreams.length)}
-                />
-              </div>
-              <div className={i === 0 ? "mt-5" : ""}>
-                <Button
-                  type="button"
-                  variant="ghost"
-                  size="icon"
-                  className="h-8 w-8"
-                  onClick={() => removeRule(i)}
-                >
-                  <Trash2 className="h-4 w-4" />
-                </Button>
+                <span className="text-xs font-medium text-muted-foreground px-1 mb-1 block">Upstreams</span>
+                <div className="flex flex-col gap-2">
+                  {rule.upstreams.map((up, j) => (
+                    <div key={j} className="flex items-center gap-2">
+                      <Select value={up.protocol} onValueChange={(val) => updateUpstreamProtocol(i, j, val)}>
+                        <SelectTrigger className="w-28 h-8 text-sm">
+                          <SelectValue />
+                        </SelectTrigger>
+                        <SelectContent>
+                          <SelectItem value="http://">http://</SelectItem>
+                          <SelectItem value="https://">https://</SelectItem>
+                        </SelectContent>
+                      </Select>
+                      <Input
+                        value={up.address}
+                        onChange={(e) => updateUpstreamAddress(i, j, e.target.value)}
+                        placeholder="10.0.0.5:8080"
+                        className="flex-1 h-8 text-sm"
+                      />
+                      <span title={rule.upstreams.length === 1 ? "At least one upstream required" : "Remove upstream"}>
+                        <Button
+                          type="button"
+                          variant="ghost"
+                          size="icon"
+                          className="h-8 w-8 text-destructive hover:text-destructive"
+                          onClick={() => removeUpstream(i, j)}
+                          disabled={rule.upstreams.length === 1}
+                        >
+                          <MinusCircle className="h-4 w-4" />
+                        </Button>
+                      </span>
+                    </div>
+                  ))}
+                  <Button type="button" variant="ghost" size="sm" onClick={() => addUpstream(i)} className="self-start">
+                    <Plus className="h-4 w-4 mr-1" />
+                    Add Upstream
+                  </Button>
+                </div>
               </div>
             </div>
           ))}
diff --git a/src/lib/caddy.ts b/src/lib/caddy.ts
index b95077a0..2b865a84 100644
--- a/src/lib/caddy.ts
+++ b/src/lib/caddy.ts
@@ -116,7 +116,7 @@ type ProxyHostMeta = {
   waf?: WafHostConfig;
   redirects?: RedirectRule[];
   rewrite?: RewriteConfig;
-  location_rules?: { path: string; upstreams: string[] }[];
+  location_rules?: LocationRule[];
 };
 
 type L4Meta = {
@@ -623,8 +623,8 @@ type BuildProxyRoutesOptions = {
   globalWaf?: WafSettings | null;
 };
 
-function buildLocationReverseProxy(
-  rule: { path: string; upstreams: string[] },
+export function buildLocationReverseProxy(
+  rule: LocationRule,
   skipHttpsValidation: boolean,
   preserveHostHeader: boolean
 ): { safePath: string; reverseProxyHandler: Record<string, unknown> } {
@@ -1042,6 +1042,8 @@ async function buildProxyRoutes(
             });
           }
 
+          // Location rules are unprotected (no forwardAuthHandler), matching the catch-all
+          // behavior when protected_paths is configured — only explicitly protected paths get auth.
           const locationRules = meta.location_rules ?? [];
           for (const rule of locationRules) {
             const { safePath, reverseProxyHandler: locationProxy } = buildLocationReverseProxy(
diff --git a/tests/unit/caddy-location-rules.test.ts b/tests/unit/caddy-location-rules.test.ts
new file mode 100644
index 00000000..8990cc36
--- /dev/null
+++ b/tests/unit/caddy-location-rules.test.ts
@@ -0,0 +1,184 @@
+/**
+ * Unit tests for buildLocationReverseProxy (src/lib/caddy.ts).
+ * Tests the Caddy config building block for location-based routing.
+ */
+import { describe, it, expect, vi } from 'vitest';
+
+// Undo the global mock so we can import the real function
+vi.unmock('@/src/lib/caddy');
+
+import { buildLocationReverseProxy } from '@/src/lib/caddy';
+
+describe('buildLocationReverseProxy', () => {
+  it('builds basic HTTP reverse proxy with single upstream', () => {
+    const { safePath, reverseProxyHandler } = buildLocationReverseProxy(
+      { path: '/api/*', upstreams: ['backend:3000'] },
+      false,
+      false
+    );
+
+    expect(safePath).toBe('/api/*');
+    expect(reverseProxyHandler).toEqual({
+      handler: 'reverse_proxy',
+      upstreams: [{ dial: 'backend:3000' }],
+    });
+  });
+
+  it('builds reverse proxy with multiple upstreams', () => {
+    const { reverseProxyHandler } = buildLocationReverseProxy(
+      { path: '/ws/*', upstreams: ['ws1:8080', 'ws2:8080', 'ws3:8080'] },
+      false,
+      false
+    );
+
+    expect(reverseProxyHandler.upstreams).toEqual([
+      { dial: 'ws1:8080' },
+      { dial: 'ws2:8080' },
+      { dial: 'ws3:8080' },
+    ]);
+  });
+
+  it('parses http:// upstream URLs into dial format', () => {
+    const { reverseProxyHandler } = buildLocationReverseProxy(
+      { path: '/api/*', upstreams: ['http://backend:3000'] },
+      false,
+      false
+    );
+
+    expect(reverseProxyHandler.upstreams).toEqual([{ dial: 'backend:3000' }]);
+    expect(reverseProxyHandler.transport).toBeUndefined();
+  });
+
+  it('parses https:// upstream URLs and adds TLS transport', () => {
+    const { reverseProxyHandler } = buildLocationReverseProxy(
+      { path: '/secure/*', upstreams: ['https://backend:443'] },
+      false,
+      false
+    );
+
+    expect(reverseProxyHandler.upstreams).toEqual([{ dial: 'backend:443' }]);
+    expect(reverseProxyHandler.transport).toEqual({
+      protocol: 'http',
+      tls: {},
+    });
+  });
+
+  it('sets insecure_skip_verify when skipHttpsValidation is true', () => {
+    const { reverseProxyHandler } = buildLocationReverseProxy(
+      { path: '/secure/*', upstreams: ['https://backend:443'] },
+      true,
+      false
+    );
+
+    expect(reverseProxyHandler.transport).toEqual({
+      protocol: 'http',
+      tls: { insecure_skip_verify: true },
+    });
+  });
+
+  it('does not add TLS transport for HTTP-only upstreams even with skipHttpsValidation', () => {
+    const { reverseProxyHandler } = buildLocationReverseProxy(
+      { path: '/api/*', upstreams: ['backend:3000'] },
+      true,
+      false
+    );
+
+    expect(reverseProxyHandler.transport).toBeUndefined();
+  });
+
+  it('preserves host header when preserveHostHeader is true', () => {
+    const { reverseProxyHandler } = buildLocationReverseProxy(
+      { path: '/api/*', upstreams: ['backend:3000'] },
+      false,
+      true
+    );
+
+    expect(reverseProxyHandler.headers).toEqual({
+      request: { set: { Host: ['{http.request.host}'] } },
+    });
+  });
+
+  it('does not set host header when preserveHostHeader is false', () => {
+    const { reverseProxyHandler } = buildLocationReverseProxy(
+      { path: '/api/*', upstreams: ['backend:3000'] },
+      false,
+      false
+    );
+
+    expect(reverseProxyHandler.headers).toBeUndefined();
+  });
+
+  it('sanitizes Caddy placeholder injection from path', () => {
+    const { safePath } = buildLocationReverseProxy(
+      { path: '/api/{http.request.uri}/*', upstreams: ['backend:3000'] },
+      false,
+      false
+    );
+
+    expect(safePath).toBe('/api//*');
+  });
+
+  it('returns empty safePath when path is entirely a placeholder', () => {
+    const { safePath } = buildLocationReverseProxy(
+      { path: '{http.request.uri}', upstreams: ['backend:3000'] },
+      false,
+      false
+    );
+
+    expect(safePath).toBe('');
+  });
+
+  it('handles mixed HTTP and HTTPS upstreams — TLS transport added', () => {
+    const { reverseProxyHandler } = buildLocationReverseProxy(
+      { path: '/mixed/*', upstreams: ['http://backend1:80', 'https://backend2:443'] },
+      false,
+      false
+    );
+
+    expect(reverseProxyHandler.upstreams).toEqual([
+      { dial: 'backend1:80' },
+      { dial: 'backend2:443' },
+    ]);
+    expect(reverseProxyHandler.transport).toEqual({
+      protocol: 'http',
+      tls: {},
+    });
+  });
+
+  it('handles HTTPS upstream with default port 443', () => {
+    const { reverseProxyHandler } = buildLocationReverseProxy(
+      { path: '/secure/*', upstreams: ['https://backend'] },
+      false,
+      false
+    );
+
+    expect(reverseProxyHandler.upstreams).toEqual([{ dial: 'backend:443' }]);
+  });
+
+  it('combines preserve host header + HTTPS transport correctly', () => {
+    const { reverseProxyHandler } = buildLocationReverseProxy(
+      { path: '/all-options/*', upstreams: ['https://backend:8443'] },
+      true,
+      true
+    );
+
+    expect(reverseProxyHandler.handler).toBe('reverse_proxy');
+    expect(reverseProxyHandler.headers).toEqual({
+      request: { set: { Host: ['{http.request.host}'] } },
+    });
+    expect(reverseProxyHandler.transport).toEqual({
+      protocol: 'http',
+      tls: { insecure_skip_verify: true },
+    });
+  });
+
+  it('handles IPv6 upstream addresses', () => {
+    const { reverseProxyHandler } = buildLocationReverseProxy(
+      { path: '/v6/*', upstreams: ['[::1]:8080'] },
+      false,
+      false
+    );
+
+    expect(reverseProxyHandler.upstreams).toEqual([{ dial: '[::1]:8080' }]);
+  });
+});