From fc6c202ea60dd34aacfcb201403c8561f56da905 Mon Sep 17 00:00:00 2001
From: Matthew Holt <mholt@users.noreply.github.com>
Date: Tue, 10 Nov 2026 13:09:00 -0700
Subject: [PATCH] rewrite: canonicalize paths after substring and regexp
 replacement

Like prefix/suffix stripping, substring and regexp path replacement can
produce a relative path or expose dot segments that later path-scoped
handlers interpret differently. Apply the existing canonicalizePath
safeguard whenever these operations change the path.

Addresses scan report 65568b0d767b733c.

Code written by GitHub Copilot (GPT-6.1 Sol, Claude Opus 5.5); reviewed by mholt.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
---
 modules/caddyhttp/rewrite/rewrite.go      | 18 ++++++---
 modules/caddyhttp/rewrite/rewrite_test.go | 45 +++++++++++++++++++++++
 2 files changed, 58 insertions(+), 5 deletions(-)

diff --git a/modules/caddyhttp/rewrite/rewrite.go b/modules/caddyhttp/rewrite/rewrite.go
index df5d32369..073d091fe 100644
--- a/modules/caddyhttp/rewrite/rewrite.go
+++ b/modules/caddyhttp/rewrite/rewrite.go
@@ -83,7 +83,8 @@ type Rewrite struct {
 	// to have that same escape at that position in order to match.
 	StripPathSuffix string `json:"strip_path_suffix,omitempty"`
 
-	// Performs substring replacements on the URI.
+	// Performs substring replacements on the URI. If a replacement
+	// changes the path, dot segments in the result are cleaned.
 	URISubstring []substrReplacer `json:"uri_substring,omitempty"`
 
 	// Performs regular expression replacements on the URI path.
@@ -581,9 +582,13 @@ func (rep substrReplacer) do(r *http.Request, repl *caddy.Replacer) {
 
 	mergeSlashes := !strings.Contains(rep.Find, "//")
 
+	oldPath := r.URL.Path
 	changePath(r, func(pathOrRawPath string) string {
 		return strings.Replace(caddyhttp.CleanPath(pathOrRawPath, mergeSlashes), find, replace, lim)
 	})
+	if r.URL.Path != oldPath {
+		canonicalizePath(r)
+	}
 
 	r.URL.RawQuery = strings.Replace(r.URL.RawQuery, find, replace, lim)
 }
@@ -605,9 +610,13 @@ func (rep regexReplacer) do(r *http.Request, repl *caddy.Replacer) {
 		return
 	}
 	replace := repl.ReplaceAll(rep.Replace, "")
+	oldPath := r.URL.Path
 	changePath(r, func(pathOrRawPath string) string {
 		return rep.re.ReplaceAllString(pathOrRawPath, replace)
 	})
+	if r.URL.Path != oldPath {
+		canonicalizePath(r)
+	}
 }
 
 func changePath(req *http.Request, newVal func(pathOrRawPath string) string) {
@@ -625,11 +634,10 @@ func changePath(req *http.Request, newVal func(pathOrRawPath string) string) {
 	}
 }
 
-// canonicalizePath anchors and cleans a stripped origin-form path while
+// canonicalizePath anchors and cleans a modified origin-form path while
 // preserving an alternate RawPath encoding when it still represents the
-// canonical Path. It is applied after prefix or suffix removal because stripping can
-// expose a relative path or dot segments that downstream handlers interpret
-// differently.
+// canonical Path. Removing or replacing path components can expose a relative
+// path or dot segments that downstream handlers interpret differently.
 func canonicalizePath(req *http.Request) {
 	p := req.URL.Path
 	if !strings.HasPrefix(p, "/") {
diff --git a/modules/caddyhttp/rewrite/rewrite_test.go b/modules/caddyhttp/rewrite/rewrite_test.go
index 41810b90f..90576d0b5 100644
--- a/modules/caddyhttp/rewrite/rewrite_test.go
+++ b/modules/caddyhttp/rewrite/rewrite_test.go
@@ -15,6 +15,7 @@
 package rewrite
 
 import (
+	"context"
 	"net/http"
 	"reflect"
 	"regexp"
@@ -22,6 +23,7 @@ import (
 	"testing"
 
 	"github.com/caddyserver/caddy/v2"
+	"github.com/caddyserver/caddy/v2/modules/caddyhttp"
 )
 
 func TestRewrite(t *testing.T) {
@@ -828,6 +830,49 @@ func TestStripPathPrefixCanonicalizesRemainder(t *testing.T) {
 	}
 }
 
+func TestPathReplacementsCanonicalizeRemainder(t *testing.T) {
+	for _, rule := range []struct {
+		name string
+		rule Rewrite
+	}{
+		{"substring", Rewrite{URISubstring: []substrReplacer{{Find: "/mount"}}}},
+		{"regexp", Rewrite{PathRegexp: []*regexReplacer{{Find: "^/mount", re: regexp.MustCompile("^/mount")}}}},
+	} {
+		t.Run(rule.name, func(t *testing.T) {
+			for _, tc := range []struct {
+				target, path, rawPath string
+			}{
+				{"/mount../secret", "/secret", ""},
+				{"/mount..%2fsecret", "/secret", ""},
+				{"/mount%2e%2e/secret", "/secret", ""},
+				{"/mount", "/", ""},
+				{"/mountain", "/ain", ""},
+				{"/mount/public", "/public", ""},
+				{"/mount/a%2Fb", "/a/b", "/a%2Fb"},
+				{"/mount/%252e%252e/secret", "/%2e%2e/secret", ""},
+				{"/mount/%ff", "/\xff", "/%ff"},
+				{"/unmatched", "/unmatched", ""},
+			} {
+				t.Run(tc.target, func(t *testing.T) {
+					r := newRequest(t, http.MethodGet, tc.target+"?keep=yes")
+					repl := caddy.NewReplacer()
+					r = r.WithContext(context.WithValue(r.Context(), caddy.ReplacerCtxKey, repl))
+					rule.rule.Rewrite(r, repl)
+					if r.URL.Path != tc.path || r.URL.RawPath != tc.rawPath {
+						t.Errorf("Path/RawPath = %q/%q; want %q/%q", r.URL.Path, r.URL.RawPath, tc.path, tc.rawPath)
+					}
+					if r.URL.RawQuery != "keep=yes" || r.RequestURI != r.URL.EscapedPath()+"?keep=yes" {
+						t.Errorf("inconsistent request target: %q", r.RequestURI)
+					}
+					if tc.path == "/secret" && !(caddyhttp.MatchPath{"/secret*"}).Match(r) {
+						t.Error("rewritten secret does not match downstream path restriction")
+					}
+				})
+			}
+		})
+	}
+}
+
 func newRequest(t *testing.T, method, uri string) *http.Request {
 	req, err := http.NewRequest(method, uri, nil)
 	if err != nil {