Merge commit from fork

* caddyhttp: canonicalise paths after prefix stripping

* caddyhttp: cover suffix stripping in path canonicalisation

caddytest/integration/rewrite_security_test.go

@@ -0,0 +1,79 @@
+package integration
+
+import (
+	"fmt"
+	"net/http"
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/caddyserver/caddy/v2/caddytest"
+)
+
+func TestStripRewritePreservesPathAuthorization(t *testing.T) {
+	root := t.TempDir()
+	for name, content := range map[string]string{
+		"index.html": "ROOT INDEX SECRET",
+		"bar":        "INNER SECRET",
+		"secret":     "DOT SEGMENT SECRET",
+	} {
+		if err := os.WriteFile(filepath.Join(root, name), []byte(content), 0o600); err != nil {
+			t.Fatal(err)
+		}
+	}
+
+	tester := caddytest.NewTester(t)
+	tester.InitServer(fmt.Sprintf(`
+	{
+		skip_install_trust
+		admin localhost:2999
+		http_port 9080
+		grace_period 1ns
+	}
+	http://localhost:9080 {
+		root * %q
+
+		handle_path /foo* {
+			basic_auth /* {
+				alice $2a$14$DCYyqvUO/PWT.UifNF0rbuRkRfjrhHViOfVfJklmqrvWUoYcTiN2q
+			}
+			file_server
+		}
+
+		handle /api* {
+			uri strip_prefix /api
+			basic_auth /secret* {
+				alice $2a$14$DCYyqvUO/PWT.UifNF0rbuRkRfjrhHViOfVfJklmqrvWUoYcTiN2q
+			}
+			file_server
+		}
+
+		handle /suffix* {
+			uri strip_suffix /suffix
+			basic_auth /* {
+				alice $2a$14$DCYyqvUO/PWT.UifNF0rbuRkRfjrhHViOfVfJklmqrvWUoYcTiN2q
+			}
+			file_server
+		}
+	}
+	`, filepath.ToSlash(root)), "caddyfile")
+
+	for _, requestPath := range []string{
+		"/foo",
+		"/foo/",
+		"/foobar",
+		"/foo/index.html",
+		"/api/secret",
+		"/api../secret",
+		"/api..%2fsecret",
+		"/suffix",
+	} {
+		t.Run(requestPath, func(t *testing.T) {
+			req, err := http.NewRequest(http.MethodGet, "http://localhost:9080"+requestPath, nil)
+			if err != nil {
+				t.Fatal(err)
+			}
+			tester.AssertResponseCode(req, http.StatusUnauthorized)
+		})
+	}
+}

modules/caddyhttp/rewrite/rewrite.go

@@ -286,18 +286,30 @@ func (rewr Rewrite) Rewrite(r *http.Request, repl *caddy.Replacer) bool {
 			prefix = "/" + prefix
 		}
 		mergeSlashes := !strings.Contains(prefix, "//")
+		stripped := false
 		changePath(r, func(escapedPath string) string {
 			escapedPath = caddyhttp.CleanPath(escapedPath, mergeSlashes)
-			return trimPathPrefix(escapedPath, prefix)
+			trimmed := trimPathPrefix(escapedPath, prefix)
+			stripped = stripped || trimmed != escapedPath
+			return trimmed
 		})
+		if stripped {
+			canonicalizePath(r)
+		}
 	}
 	if rewr.StripPathSuffix != "" {
 		suffix := repl.ReplaceAll(rewr.StripPathSuffix, "")
 		mergeSlashes := !strings.Contains(suffix, "//")
+		stripped := false
 		changePath(r, func(escapedPath string) string {
 			escapedPath = caddyhttp.CleanPath(escapedPath, mergeSlashes)
-			return trimPathSuffix(escapedPath, suffix)
+			trimmed := trimPathSuffix(escapedPath, suffix)
+			stripped = stripped || trimmed != escapedPath
+			return trimmed
 		})
+		if stripped {
+			canonicalizePath(r)
+		}
 	}
 
 	// substring replacements in URI
@@ -591,6 +603,34 @@ func changePath(req *http.Request, newVal func(pathOrRawPath string) string) {
 	}
 }
 
+// canonicalizePath anchors and cleans a stripped 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.
+func canonicalizePath(req *http.Request) {
+	p := req.URL.Path
+	if !strings.HasPrefix(p, "/") {
+		p = "/" + p
+	}
+	cleaned := caddyhttp.CleanPath(defaultEscapedPath(p), false)
+	p, _ = url.PathUnescape(cleaned) // defaultEscapedPath always returns valid escapes
+
+	rawPath := req.URL.RawPath
+	if rawPath != "" {
+		if !strings.HasPrefix(rawPath, "/") {
+			rawPath = "/" + rawPath
+		}
+		decoded, err := url.PathUnescape(rawPath)
+		if err != nil || decoded != p || rawPath == defaultEscapedPath(p) {
+			rawPath = ""
+		}
+	}
+
+	req.URL.Path = p
+	req.URL.RawPath = rawPath
+}
+
 // defaultEscapedPath returns the canonical percent-encoding of p, matching
 // what net/url.URL.EscapedPath() produces when RawPath is empty. It mirrors
 // the comparison net/url.URL.setPath uses to decide whether RawPath is needed.

modules/caddyhttp/rewrite/rewrite_test.go

@@ -250,7 +250,7 @@ func TestRewrite(t *testing.T) {
 		{
 			rule:   Rewrite{StripPathPrefix: "/prefix"},
 			input:  newRequest(t, "GET", "/prefix"),
-			expect: newRequest(t, "GET", ""),
+			expect: newRequest(t, "GET", "/"),
 		},
 		{
 			rule:   Rewrite{StripPathPrefix: "/prefix"},
@@ -315,6 +315,11 @@ func TestRewrite(t *testing.T) {
 			input:  newRequest(t, "GET", "/foo/bar"),
 			expect: newRequest(t, "GET", "/foo/bar"),
 		},
+		{
+			rule:   Rewrite{StripPathSuffix: "/suffix"},
+			input:  newRequest(t, "GET", "/suffix"),
+			expect: newRequest(t, "GET", "/"),
+		},
 		{
 			rule:   Rewrite{StripPathSuffix: "suffix"},
 			input:  newRequest(t, "GET", "/foo/bar/suffix"),
@@ -673,6 +678,40 @@ func TestQueryOpsReplaceScopedToKey(t *testing.T) {
 	}
 }
 
+func TestStripPathPrefixCanonicalizesRemainder(t *testing.T) {
+	for _, tc := range []struct {
+		name    string
+		target  string
+		prefix  string
+		path    string
+		rawPath string
+	}{
+		{name: "empty remainder", target: "/mount", prefix: "/mount", path: "/"},
+		{name: "mid-segment remainder", target: "/mountain", prefix: "/mount", path: "/ain"},
+		{name: "parent segment", target: "/mount../document", prefix: "/mount", path: "/document"},
+		{name: "current segment", target: "/mount./document/", prefix: "/mount", path: "/document/"},
+		{name: "encoded parent segment", target: "/mount%2e%2e/document", prefix: "/mount", path: "/document"},
+		{name: "encoded leading separator", target: "/mount%2Fdocument", prefix: "/mount", path: "/document"},
+		{name: "alternate separator encoding", target: "/mount/a%2Fb", prefix: "/mount", path: "/a/b", rawPath: "/a%2Fb"},
+		{name: "double-encoded parent segment", target: "/mount/%252e%252e/document", prefix: "/mount", path: "/%2e%2e/document"},
+		{name: "non-UTF-8 byte", target: "/mount/%ff", prefix: "/mount", path: "/\xff", rawPath: "/%ff"},
+		{name: "repeated separators", target: "/mount//a//b/", prefix: "/mount//", path: "/a//b/"},
+		{name: "ordinary remainder", target: "/mount/document", prefix: "/mount", path: "/document"},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			r := newRequest(t, http.MethodGet, tc.target+"?keep=yes")
+			Rewrite{StripPathPrefix: tc.prefix}.Rewrite(r, caddy.NewReplacer())
+
+			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("query or request target changed inconsistently: %q", r.RequestURI)
+			}
+		})
+	}
+}
+
 func newRequest(t *testing.T, method, uri string) *http.Request {
 	req, err := http.NewRequest(method, uri, nil)
 	if err != nil {