fix: authorize the destination collection a copy or move replaces

A COPY or MOVE onto an existing destination replaces it, which destroys
everything the destination collection held. Only the source subtree was
authorized per descendant, and the destination itself was authorized as an
update, so a rule restricting a subtree held against DELETE but not against
an overwrite of the collection above it. Removing a collection is now
authorized the way DELETE on it would be, on the collection and on every
path beneath it. Writing over a file stays an update.

README.md

@@ -121,6 +121,9 @@ directory: /data
 # permissions. For example, to allow to read and create, set "RC". Default is "R".
 # LOCK counts as a write: it needs U on a path that exists and C on one that does
 # not, since locking a path that does not exist creates it.
+# Being overwritten counts as well: a COPY or MOVE onto an existing file replaces
+# it and needs U, while one onto an existing collection removes everything it
+# holds and needs D, on the collection and on every path under it.
 permissions: R
 
 # The default permissions rules for users. Default is none. Rules are applied
@@ -244,6 +247,8 @@ A `regex` rule is matched literally against the path, and gets none of the above
 
 Rules apply to every path an operation touches, not only the one it names. Collection listings leave out entries the rules deny, copying a collection leaves those entries behind, and a `MOVE` or `DELETE` that would act on a denied descendant is refused outright.
 
+Overwriting a destination is authorized for what it destroys. A `COPY` or `MOVE` onto an existing destination replaces it: RFC4918 has `MOVE` perform a `DELETE` with `Depth: infinity` on the destination first, and requires an overwritten collection to end up with exactly the membership the source had, so either way whatever was there is gone. Replacing a file needs `U` on it, the same permission `PUT` needs. Replacing a collection removes everything it holds, so it needs `D` on that collection and on every path beneath it: a rule withholding `D` anywhere under a destination refuses the overwrite outright, even where it grants `C` and `U`.
+
 Rules follow the case sensitivity of the file system, which each served directory is probed for at startup. Where names are case-insensitive, as on APFS and NTFS, `path: /secret/` also covers `/SECRET/`, a `regex` is matched against the folded path as well as the path as written, and Unicode normal forms count as one name. Elsewhere rules are matched exactly, since `/secret` and `/SECRET` are then different directories.
 
 ### CORS
@@ -270,11 +275,13 @@ location / {
   proxy_set_header Host $host;
   proxy_redirect off;
 
-  # Ensure COPY and MOVE commands work. Change https://example.com to the
-  # correct address where the WebDAV server will be deployed at.
+  # Ensure COPY and MOVE commands work by rewriting the Destination header to
+  # contain only the path, e.g. /test.txt. Note that the captured group already
+  # includes the leading slash: adding another one would produce a Destination
+  # such as //test.txt, which is parsed as a host name and rejected.
   set $dest $http_destination;
-  if ($http_destination ~ "^https://example.com(?<path>(.+))") {
-    set $dest /$path;
+  if ($http_destination ~ "^https?://[^/]+(?<path>/.*)$") {
+    set $dest $path;
   }
   proxy_set_header Destination $dest;
 }
@@ -299,6 +306,41 @@ example.com {
 }
 ```
 
+#### Serving Under a Subpath
+
+If the server is not served from the root of the domain, do not strip the subpath in the reverse proxy. The server needs to see it: `PROPFIND` responses contain the full path of each resource, and clients reject the ones that fall outside of the URL they requested. Pass the subpath through and set [`prefix`](#configuration) accordingly, so that the server strips it itself and adds it back to the responses:
+
+```yaml
+prefix: /webdav
+```
+
+With Caddy, that means using `handle` instead of `handle_path`, as the latter strips the matched prefix before proxying:
+
+```Caddyfile
+example.com {
+    @hasDest header_regexp dest ^https?://[^/]+(.*)$
+    header @hasDest Destination {re.dest.1}
+
+    handle /webdav* {
+        reverse_proxy 127.0.0.1:6065 {
+            header_up X-Real-IP {remote_host}
+            header_up REMOTE-HOST {remote_host}
+        }
+    }
+}
+```
+
+With Nginx, use a `location` block for the subpath and keep `proxy_pass` without a trailing path, as a trailing path would replace the prefix:
+
+```nginx
+location /webdav {
+  proxy_pass http://127.0.0.1:6065;
+  # ... the remaining headers, as above.
+}
+```
+
+Both the request path and the `Destination` header must carry the prefix. A request without it is answered with `400 Bad Request`.
+
 ## Examples
 
 ### Systemd

go.mod

@@ -12,10 +12,10 @@ require (
 	github.com/stretchr/testify v1.12.1
 	github.com/studio-b12/gowebdav v0.13.0
 	go.uber.org/zap v1.28.0
-	golang.org/x/crypto v0.55.0
-	golang.org/x/crypto/x509roots/fallback v0.0.0-20260826144058-afebf4cb4efb
-	golang.org/x/net v0.58.0
-	golang.org/x/text v0.41.0
+	golang.org/x/crypto v0.57.0
+	golang.org/x/crypto/x509roots/fallback v0.0.0-20260920014000-1f7c531b64a1
+	golang.org/x/net v0.59.0
+	golang.org/x/text v0.42.0
 )
 
 require (
@@ -29,5 +29,5 @@ require (
 	github.com/subosito/gotenv v1.6.0 // indirect
 	go.uber.org/multierr v1.11.0 // indirect
 	go.yaml.in/yaml/v3 v3.0.5 // indirect
-	golang.org/x/sys v0.47.0 // indirect
+	golang.org/x/sys v0.48.0 // indirect
 )

go.sum

@@ -52,14 +52,14 @@ go.uber.org/zap v1.28.0/go.mod h1:rDLpOi171uODNm/mxFcuYWxDsqWSAVkFdX4XojSKg/Q=
 go.yaml.in/yaml/v3 v3.0.4/go.mod h1:DhzuOOF2ATzADvBadXxruRBLzYTpT36CKvDb3+aBEFg=
 go.yaml.in/yaml/v3 v3.0.5 h1:N6y/pJk8buWs9NY5ERU2HSMfm+IuD/OtfdAnq6kESPw=
 go.yaml.in/yaml/v3 v3.0.5/go.mod h1:HVTZu1O7/Vkt2N+BFy8Zza+lnLsABggaTM2ZpNIGuKg=
-golang.org/x/crypto v0.55.0 h1:+KWHjbgOaAQ66dh/YlkZKHlz9ZUlq61AFirAR9ntP8M=
-golang.org/x/crypto v0.55.0/go.mod h1:uq0V9dE/fzQuJtbnL+2EhWOE63vo164FY8xqEnV9xis=
-golang.org/x/crypto/x509roots/fallback v0.0.0-20260826144058-afebf4cb4efb h1:3OQBwC/IAO9+1/yWsvWkE1Lw5InpHGlZxXZHUcWBouA=
-golang.org/x/crypto/x509roots/fallback v0.0.0-20260826144058-afebf4cb4efb/go.mod h1:HPze8vhfG6fO06AM+VSvxRm4E3+5Yk375mgrJ5M2z1E=
-golang.org/x/net v0.58.0 h1:ynWG7rqYi4ccpTEuPZ2QGWHktVEM9DMCj9yzDE0Q7To=
-golang.org/x/net v0.58.0/go.mod h1:YwCddHnFlT7eLQqVprV19OnhLGtc5xOKgE0RyqgfWAU=
-golang.org/x/sys v0.47.0 h1:o7XGOvZQCADBQQ4Y7VNq2dRWQR7JmOUW8Kxx4ZsNgWs=
-golang.org/x/sys v0.47.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw=
-golang.org/x/text v0.41.0 h1:vz/seA0lnX87Othu2f/0L24RcgrXD9/YFTSuGjj3rH8=
-golang.org/x/text v0.41.0/go.mod h1:jvf1O8ajNzZqhSrQBPbutR/EB83Cc0CFrezNQIwbb5M=
+golang.org/x/crypto v0.57.0 h1:3ZVCjf8Ggz7zneR/EHRVx68Ctf+2pmIMP2UFhh9cC6M=
+golang.org/x/crypto v0.57.0/go.mod h1:Fdz0i5U6CoizGwLda9DttjSk6qlZo25zYNtR+ycvuZA=
+golang.org/x/crypto/x509roots/fallback v0.0.0-20260920014000-1f7c531b64a1 h1:n8oH0y5uJlfek8vblc51aexgj0qKjh95bYIANGCaxoI=
+golang.org/x/crypto/x509roots/fallback v0.0.0-20260920014000-1f7c531b64a1/go.mod h1:HPze8vhfG6fO06AM+VSvxRm4E3+5Yk375mgrJ5M2z1E=
+golang.org/x/net v0.59.0 h1:5zfYln+w5XCxwrnMMJPufRgNoXEaGxl0wo5GqPXyues=
+golang.org/x/net v0.59.0/go.mod h1:2DA/G1UfVbCpQPeWTmMPGY7Cs2PkBkwu743bVX5PIVg=
+golang.org/x/sys v0.48.0 h1:bbX/i/6MgT9BVLM9RT1thmxL04yeTAhbEz4SyadbXoo=
+golang.org/x/sys v0.48.0/go.mod h1:hNLxWAXmnKAxqDtdwIYC4bM9oQPEecfsnNMuSxOs3og=
+golang.org/x/text v0.42.0 h1:JbOZXgfeCPU9gacVtYliJqOhD+zhrEqK4LfdpmlUZqI=
+golang.org/x/text v0.42.0/go.mod h1:ojzP1Z+2QtioaF8DTtO8K5q7JWVVYwZKenzujK0Zd0E=
 gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=

lib/handler.go

@@ -169,8 +169,9 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	// MOVE and DELETE act on a whole subtree in one call, so every descendant
-	// needs authorizing here. COPY and PROPFIND go through permFS instead.
+	// MOVE and DELETE act on a whole source subtree in one call, so every
+	// descendant needs authorizing here. Reading COPY and PROPFIND out of the
+	// source goes through permFS instead.
 	if r.Method == "MOVE" || r.Method == "DELETE" {
 		ok, err := user.fs.allowedThroughout(r.Context(), req.path, func(p Permissions) bool {
 			return p.Allowed(req, fileExists)
@@ -188,6 +189,50 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 		}
 	}
 
+	// Excerpt from RFC4918, section 9.9.3, on MOVE:
+	//
+	//		If a resource exists at the destination and the Overwrite header is
+	//		"T", then prior to performing the move, the server MUST perform a
+	//		DELETE with "Depth: infinity" on the destination resource.
+	//
+	// And from section 9.8.4, on COPY:
+	//
+	//		When a collection is overwritten, the membership of the destination
+	//		collection after the successful COPY request MUST be the same
+	//		membership as the source collection immediately before the COPY.
+	//
+	// Either way whatever the destination collection held is gone, which
+	// golang.org/x/net/webdav carries out as a RemoveAll before the rename or
+	// the copy. Writing over a file is an update, already authorized by Allowed,
+	// but removing a collection takes everything under it. Nothing writes to
+	// those descendants, they are only destroyed, so authorize the destination
+	// the way DELETE on that collection would be.
+	if (r.Method == "MOVE" || r.Method == "COPY") && req.destination != "" {
+		info, err := user.fs.Stat(r.Context(), req.destination)
+		if err == nil && info.IsDir() {
+			deletable := func(p Permissions) bool { return p.Delete }
+
+			// allowedThroughout reaches descendants only, and the destination
+			// check in Allowed covers the collection itself as an update rather
+			// than a delete, so that is checked here.
+			ok := user.allowedAt(req.destination, deletable)
+			if ok {
+				ok, err = user.fs.allowedThroughout(r.Context(), req.destination, deletable)
+				if err != nil {
+					lZap.Error("could not authorize destination subtree", zap.String("destination", req.destination), zap.Error(err))
+					w.WriteHeader(http.StatusForbidden)
+					return
+				}
+			}
+
+			if !ok {
+				lZap.Info("denied by a rule on the destination collection", zap.String("method", r.Method), zap.String("destination", req.destination))
+				w.WriteHeader(http.StatusForbidden)
+				return
+			}
+		}
+	}
+
 	if r.Method == "HEAD" {
 		w = responseWriterNoBody{w}
 	}

lib/handler_test.go

@@ -1462,6 +1462,179 @@ rules:
 	require.FileExists(t, filepath.Join(dirA, "secret", "flag.txt"))
 }
 
+// TestServerRulesDestinationOverwriteDescendants covers a COPY or MOVE onto an
+// existing collection, which replaces it: the server deletes the destination
+// with "Depth: infinity" first, reaching descendants that authorizing only the
+// destination itself let it destroy.
+func TestServerRulesDestinationOverwriteDescendants(t *testing.T) {
+	t.Parallel()
+
+	dir := makeTestDirectory(t, map[string][]byte{
+		"shared/protected/confidential.txt": []byte("top secret"),
+		"empty/.keep":                       []byte(""),
+	})
+
+	srv := makeTestServer(t, fmt.Sprintf(`
+directory: %s
+permissions: CRUD
+rules:
+  - path: "/shared/protected/"
+    permissions: R
+`, dir))
+	defer srv.Close()
+
+	confidential := filepath.Join(dir, "shared", "protected", "confidential.txt")
+
+	// Controls: the rule holds when the request names the denied subtree.
+	code, _ := doRequest(t, "DELETE", srv.URL+"/shared/protected/confidential.txt", "", "", nil, "")
+	require.Equal(t, http.StatusForbidden, code)
+
+	code, _ = doRequest(t, "DELETE", srv.URL+"/shared/", "", "", nil, "")
+	require.Equal(t, http.StatusForbidden, code)
+
+	// Overwriting the collection above the rule destroys the denied subtree, so
+	// it is refused for the same reason DELETE is.
+	code, _ = doRequest(t, "MOVE", srv.URL+"/empty/", "", "", map[string]string{
+		"Destination": srv.URL + "/shared/",
+		"Overwrite":   "T",
+	}, "")
+	require.Equal(t, http.StatusForbidden, code)
+
+	// COPY overwrites unless the header says otherwise, so it needs no Overwrite.
+	code, _ = doRequest(t, "COPY", srv.URL+"/empty/", "", "", map[string]string{
+		"Destination": srv.URL + "/shared/",
+		"Depth":       "infinity",
+	}, "")
+	require.Equal(t, http.StatusForbidden, code)
+
+	require.FileExists(t, confidential)
+}
+
+// TestServerRulesDestinationOverwriteDescendantsMultiDir is the same defect
+// across mounts, where the destination collection lives under another mount.
+func TestServerRulesDestinationOverwriteDescendantsMultiDir(t *testing.T) {
+	t.Parallel()
+
+	dirA := makeTestDirectory(t, map[string][]byte{"shared/protected/flag.txt": []byte("mount secret")})
+	dirB := makeTestDirectory(t, map[string][]byte{"empty/.keep": []byte("")})
+
+	srv := makeTestServer(t, fmt.Sprintf(`
+permissions: CRUD
+directories:
+  - name: alpha
+    path: %s
+  - name: beta
+    path: %s
+rules:
+  - path: "/alpha/shared/protected/"
+    permissions: R
+`, dirA, dirB))
+	defer srv.Close()
+
+	code, _ := doRequest(t, "MOVE", srv.URL+"/beta/empty/", "", "", map[string]string{
+		"Destination": srv.URL + "/alpha/shared/",
+		"Overwrite":   "T",
+	}, "")
+	require.Equal(t, http.StatusForbidden, code)
+
+	code, _ = doRequest(t, "COPY", srv.URL+"/beta/empty/", "", "", map[string]string{
+		"Destination": srv.URL + "/alpha/shared/",
+		"Depth":       "infinity",
+	}, "")
+	require.Equal(t, http.StatusForbidden, code)
+
+	require.FileExists(t, filepath.Join(dirA, "shared", "protected", "flag.txt"))
+}
+
+// TestServerRulesDestinationOverwriteRequiresDelete covers the permission class
+// the overwrite is authorized under. Removing a collection is delete-class, so a
+// rule that grants writes but withholds D refuses the overwrite, whether it
+// governs the destination itself or something beneath it.
+func TestServerRulesDestinationOverwriteRequiresDelete(t *testing.T) {
+	t.Parallel()
+
+	dir := makeTestDirectory(t, map[string][]byte{
+		"empty/.keep":       []byte(""),
+		"empty2/.keep":      []byte(""),
+		"nodelete/note.txt": []byte("kept by the rule on the collection"),
+		"shared/keep/n.txt": []byte("kept by the rule on a descendant"),
+	})
+
+	srv := makeTestServer(t, fmt.Sprintf(`
+directory: %s
+permissions: CRUD
+rules:
+  - path: "/nodelete/"
+    permissions: CRU
+  - path: "/shared/keep/"
+    permissions: CRU
+`, dir))
+	defer srv.Close()
+
+	// The rule governs the destination collection itself.
+	code, _ := doRequest(t, "MOVE", srv.URL+"/empty/", "", "", map[string]string{
+		"Destination": srv.URL + "/nodelete/",
+		"Overwrite":   "T",
+	}, "")
+	require.Equal(t, http.StatusForbidden, code)
+	require.FileExists(t, filepath.Join(dir, "nodelete", "note.txt"))
+
+	// The rule governs a descendant of the destination collection.
+	code, _ = doRequest(t, "MOVE", srv.URL+"/empty2/", "", "", map[string]string{
+		"Destination": srv.URL + "/shared/",
+		"Overwrite":   "T",
+	}, "")
+	require.Equal(t, http.StatusForbidden, code)
+	require.FileExists(t, filepath.Join(dir, "shared", "keep", "n.txt"))
+}
+
+// TestServerDestinationOverwriteAllowed pins what overwriting a destination is
+// still allowed to do, so the delete-class check on collections does not spread
+// to writing over a file. Replacing a file is update-class, the same class PUT
+// over an existing file needs, which clients rely on when they save by writing a
+// temporary file and moving it over the target.
+func TestServerDestinationOverwriteAllowed(t *testing.T) {
+	t.Parallel()
+
+	dir := makeTestDirectory(t, map[string][]byte{
+		"empty/.keep":       []byte(""),
+		"empty2/.keep":      []byte(""),
+		"plain/note.txt":    []byte("plain"),
+		"updatable/doc.txt": []byte("updatable"),
+		"source.txt":        []byte("source"),
+	})
+
+	srv := makeTestServer(t, fmt.Sprintf(`
+directory: %s
+permissions: CRUD
+rules:
+  - path: "/updatable/doc.txt"
+    permissions: RU
+`, dir))
+	defer srv.Close()
+
+	// No rule withholds D anywhere under the destination.
+	code, _ := doRequest(t, "MOVE", srv.URL+"/empty/", "", "", map[string]string{
+		"Destination": srv.URL + "/plain/",
+		"Overwrite":   "T",
+	}, "")
+	require.Equal(t, http.StatusNoContent, code)
+
+	// A destination that does not exist is created, not overwritten.
+	code, _ = doRequest(t, "MOVE", srv.URL+"/empty2/", "", "", map[string]string{
+		"Destination": srv.URL + "/fresh/",
+		"Overwrite":   "T",
+	}, "")
+	require.Equal(t, http.StatusCreated, code)
+
+	// A file destination needs only the U its rule grants, not D.
+	code, _ = doRequest(t, "MOVE", srv.URL+"/source.txt", "", "", map[string]string{
+		"Destination": srv.URL + "/updatable/doc.txt",
+		"Overwrite":   "T",
+	}, "")
+	require.Equal(t, http.StatusNoContent, code)
+}
+
 // TestServerLockRequiresWritePermission covers LOCK being authorized by any
 // permission at all, which let a read-only user create a file by locking a
 // missing path, and hold a write lock that blocks legitimate writers.