From 894610d59e8eaf58f39697ad4c2124beab726c43 Mon Sep 17 00:00:00 2001 From: ZDDC Date: Thu, 4 Jun 2026 21:01:51 -0500 Subject: [PATCH] feat(server): admin folder move + recursive delete (file API) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Directory MOVE and DELETE were hard-rejected with 409 "not supported" for everyone, so a folder could never be renamed, relocated, or removed — even in admin mode. The browse menu offered Rename/Delete on folder rows, but they failed at the server. This is exactly the restructuring admin mode exists for (e.g. doing a layout migration by hand instead of a script). serveFileMove: a directory source is now allowed when the principal is an active admin (zddc.IsSubtreeAdmin) over BOTH the source subtree and the destination's parent — a root admin covers all; a subtree admin within scope. os.Rename relocates the whole subtree (bypassing the per-file WORM/ACL gates on its contents, which is the point), and a move into the directory's own descendant is refused (409). File moves are unchanged. serveFileDelete: a directory target is now allowed for an active admin over that subtree and removes it recursively (os.RemoveAll). Non-admins get 403. Both relax the trailing-slash guard (the browse client sends folder ops with a trailing slash) and decide file-vs-directory by stat. Directory ops skip the If-Match precondition (a directory carries no ETag). Recursive deletes are audited with a "(recursive)" marker. Non-admin directory ops now return 403 rather than the old blanket 409. Co-Authored-By: Claude Opus 4.8 (1M context) --- zddc/internal/handler/fileapi.go | 76 +++++++++++++++++------ zddc/internal/handler/fileapi_test.go | 88 +++++++++++++++++++++++++-- 2 files changed, 143 insertions(+), 21 deletions(-) diff --git a/zddc/internal/handler/fileapi.go b/zddc/internal/handler/fileapi.go index 364a9f9..49bd173 100644 --- a/zddc/internal/handler/fileapi.go +++ b/zddc/internal/handler/fileapi.go @@ -497,10 +497,8 @@ func serveFileDelete(cfg config.Config, w http.ResponseWriter, r *http.Request) http.Error(w, msg, status) return } - if strings.HasSuffix(cleanURL, "/") { - http.Error(w, "DELETE must target a file, not a directory", http.StatusBadRequest) - return - } + // (Directory vs file is decided by stat below; the client sends a folder + // DELETE with a trailing slash. A directory delete is admin-gated.) // Register rows are real files — a DELETE targets them directly with // the normal ACL gate. (Deleting an ssr/.yaml de-registers the @@ -517,7 +515,32 @@ func serveFileDelete(cfg config.Config, w http.ResponseWriter, r *http.Request) return } if info.IsDir() { - http.Error(w, "Conflict — DELETE of directories is not supported", http.StatusConflict) + // Directory delete is recursive (os.RemoveAll), which bypasses the + // per-file WORM/delete gates protecting the contents — so it's + // admin-only: an active admin over this subtree (a root admin, or a + // subtree admin within scope). This is the "admin mode exists for + // restructuring" capability. + p := PrincipalFromContext(r) + if !zddc.IsSubtreeAdmin(cfg.Root, abs, p) { + http.Error(w, "Forbidden — deleting a directory requires admin authority over it", http.StatusForbidden) + return + } + if err := os.RemoveAll(abs); err != nil { + auditFile(r, "delete", cleanURL+" (recursive)", http.StatusInternalServerError, 0, err) + http.Error(w, "Internal Server Error", http.StatusInternalServerError) + return + } + etagCacheM.Delete(abs) + purgeConverted(abs) + w.Header().Set("X-ZDDC-Source", "fileapi:delete-dir") + w.WriteHeader(http.StatusNoContent) + auditFile(r, "delete", cleanURL+" (recursive)", http.StatusNoContent, 0, nil) + return + } + + // File delete: a trailing slash is a directory URL — reject the mismatch. + if strings.HasSuffix(cleanURL, "/") { + http.Error(w, "DELETE must target a file, not a directory", http.StatusBadRequest) return } @@ -571,10 +594,9 @@ func serveFileMove(cfg config.Config, w http.ResponseWriter, r *http.Request) { http.Error(w, msg, status) return } - if strings.HasSuffix(srcURL, "/") { - http.Error(w, "MOVE source must be a file path", http.StatusBadRequest) - return - } + // (A trailing slash on src/dst signals a directory target; we no longer + // reject it here — file-vs-directory is decided by stat below, and a + // directory move is admin-gated.) dstHeader := r.Header.Get(headerDestination) if dstHeader == "" { @@ -590,10 +612,6 @@ func serveFileMove(cfg config.Config, w http.ResponseWriter, r *http.Request) { http.Error(w, "destination: "+msg, status) return } - if strings.HasSuffix(dstURL, "/") { - http.Error(w, "MOVE destination must be a file path", http.StatusBadRequest) - return - } // A move whose destination introduces a new party folder under a // party_source peer requires the party to be registered. if rejected, why, _ := partySourceGate(cfg.Root, dstAbs); rejected { @@ -619,8 +637,28 @@ func serveFileMove(cfg config.Config, w http.ResponseWriter, r *http.Request) { } return } - if srcInfo.IsDir() { - http.Error(w, "Conflict — MOVE of directories is not supported", http.StatusConflict) + isDir := srcInfo.IsDir() + if isDir { + // Directory moves relocate the whole subtree with one os.Rename, + // which sidesteps the per-file WORM/ACL gates protecting the + // descendants — so they're admin-only: an active admin over BOTH the + // source subtree and the destination's parent (a root admin covers + // all; a subtree admin within their own scope). This is the "admin + // mode exists for restructuring" capability. + p := PrincipalFromContext(r) + if !zddc.IsSubtreeAdmin(cfg.Root, srcAbs, p) || + !zddc.IsSubtreeAdmin(cfg.Root, filepath.Dir(dstAbs), p) { + http.Error(w, "Forbidden — moving a directory requires admin authority over the source and destination", http.StatusForbidden) + return + } + // Refuse moving a directory into itself or one of its descendants. + if dstAbs == srcAbs || strings.HasPrefix(dstAbs, srcAbs+string(filepath.Separator)) { + http.Error(w, "Conflict — cannot move a directory into itself", http.StatusConflict) + return + } + } else if strings.HasSuffix(dstURL, "/") { + // A file move must target a file path, not a directory URL. + http.Error(w, "destination: MOVE of a file must target a file path", http.StatusBadRequest) return } @@ -643,8 +681,12 @@ func serveFileMove(cfg config.Config, w http.ResponseWriter, r *http.Request) { if !authorizeAction(cfg, w, r, dstAbs, dstURL, policy.ActionCreate) { return } - if !checkIfMatch(w, r, srcAbs) { - return + // If-Match concurrency applies to the source bytes — only meaningful for + // a file. A directory carries no ETag, so skip the precondition. + if !isDir { + if !checkIfMatch(w, r, srcAbs) { + return + } } // Ensure destination's canonical ancestors are created (with auto-own diff --git a/zddc/internal/handler/fileapi_test.go b/zddc/internal/handler/fileapi_test.go index aea37cd..3023a0e 100644 --- a/zddc/internal/handler/fileapi_test.go +++ b/zddc/internal/handler/fileapi_test.go @@ -284,11 +284,34 @@ func TestFileAPI_DeleteMissing404(t *testing.T) { } } -func TestFileAPI_DeleteDirectoryConflict(t *testing.T) { +// Directory delete is admin-only and recursive. A non-admin (elevated but not +// named in admins:) is forbidden; an admin recursively removes the subtree. +func TestFileAPI_DeleteDirectoryNonAdminForbidden(t *testing.T) { _, do, _ := fileAPITestSetup(t, []string{"Incoming/sub"}, nil) - rec := do(http.MethodDelete, "/Incoming/sub", "alice@example.com", nil, nil) - if rec.Code != http.StatusConflict { - t.Fatalf("want 409, got %d: %s", rec.Code, rec.Body.String()) + rec := do(http.MethodDelete, "/Incoming/sub/", "alice@example.com", nil, nil) + if rec.Code != http.StatusForbidden { + t.Fatalf("want 403, got %d: %s", rec.Code, rec.Body.String()) + } +} + +func TestFileAPI_DeleteDirectoryAdminRecursive(t *testing.T) { + _, do, root := fileAPITestSetup(t, []string{"Incoming/sub"}, map[string]string{ + "Incoming/sub/a.txt": "one", + "Incoming/sub/deep/b.txt": "two", + }) + // Promote alice to root admin. + if err := os.WriteFile(filepath.Join(root, ".zddc"), + []byte("acl:\n permissions:\n \"*@example.com\": rwcd\nadmins:\n - alice@example.com\n"), 0o644); err != nil { + t.Fatalf("rewrite root .zddc: %v", err) + } + zddc.InvalidateCache(root) + + rec := do(http.MethodDelete, "/Incoming/sub/", "alice@example.com", nil, nil) + if rec.Code != http.StatusNoContent { + t.Fatalf("want 204, got %d: %s", rec.Code, rec.Body.String()) + } + if _, err := os.Stat(filepath.Join(root, "Incoming/sub")); !os.IsNotExist(err) { + t.Fatalf("dir should be gone recursively, err=%v", err) } } @@ -319,6 +342,63 @@ func TestFileAPI_MoveRenames(t *testing.T) { } } +// Directory move is admin-only and relocates the whole subtree. A non-admin +// (elevated but not in admins:) is forbidden; an admin renames/relocates it. +func TestFileAPI_MoveDirectoryNonAdminForbidden(t *testing.T) { + _, do, _ := fileAPITestSetup(t, []string{"Docs/sub"}, nil) + rec := do(http.MethodPost, "/Docs/sub/", "alice@example.com", nil, map[string]string{ + "X-ZDDC-Op": "move", + "X-ZDDC-Destination": "/Docs/renamed/", + }) + if rec.Code != http.StatusForbidden { + t.Fatalf("want 403, got %d: %s", rec.Code, rec.Body.String()) + } +} + +func TestFileAPI_MoveDirectoryAdmin(t *testing.T) { + _, do, root := fileAPITestSetup(t, []string{"Docs/sub"}, map[string]string{ + "Docs/sub/a.txt": "x", + "Docs/sub/deep/b.txt": "y", + }) + if err := os.WriteFile(filepath.Join(root, ".zddc"), + []byte("acl:\n permissions:\n \"*@example.com\": rwcd\nadmins:\n - alice@example.com\n"), 0o644); err != nil { + t.Fatalf("rewrite root .zddc: %v", err) + } + zddc.InvalidateCache(root) + + rec := do(http.MethodPost, "/Docs/sub/", "alice@example.com", nil, map[string]string{ + "X-ZDDC-Op": "move", + "X-ZDDC-Destination": "/Docs/renamed/", + }) + if rec.Code != http.StatusOK { + t.Fatalf("want 200, got %d: %s", rec.Code, rec.Body.String()) + } + if _, err := os.Stat(filepath.Join(root, "Docs/sub")); !os.IsNotExist(err) { + t.Fatalf("source dir should be gone, err=%v", err) + } + if b, err := os.ReadFile(filepath.Join(root, "Docs/renamed/deep/b.txt")); err != nil || string(b) != "y" { + t.Fatalf("moved subtree content missing: b=%q err=%v", b, err) + } +} + +// Refuse moving a directory into itself or a descendant (would orphan the tree). +func TestFileAPI_MoveDirectoryIntoItself(t *testing.T) { + _, do, root := fileAPITestSetup(t, []string{"Docs/sub"}, nil) + if err := os.WriteFile(filepath.Join(root, ".zddc"), + []byte("acl:\n permissions:\n \"*@example.com\": rwcd\nadmins:\n - alice@example.com\n"), 0o644); err != nil { + t.Fatalf("rewrite root .zddc: %v", err) + } + zddc.InvalidateCache(root) + + rec := do(http.MethodPost, "/Docs/sub/", "alice@example.com", nil, map[string]string{ + "X-ZDDC-Op": "move", + "X-ZDDC-Destination": "/Docs/sub/inner/", + }) + if rec.Code != http.StatusConflict { + t.Fatalf("want 409, got %d: %s", rec.Code, rec.Body.String()) + } +} + func TestFileAPI_MoveDestinationExistsConflict(t *testing.T) { _, do, _ := fileAPITestSetup(t, nil, map[string]string{ "Incoming/a.txt": "a",