From 0bdc206d6c552e39b5c05a7e2b686b57035c7d02 Mon Sep 17 00:00:00 2001 From: OpenClaw Agent Date: Tue, 22 Sep 2026 00:17:25 +0800 Subject: [PATCH] test: harden multipart upload compatibility --- chfs-ng/internal/api/server.go | 30 +++- chfs-ng/internal/api/upload_compat_test.go | 156 +++++++++++++++++++++ 2 files changed, 184 insertions(+), 2 deletions(-) create mode 100644 chfs-ng/internal/api/upload_compat_test.go diff --git a/chfs-ng/internal/api/server.go b/chfs-ng/internal/api/server.go index a6f262c..b50d47e 100644 --- a/chfs-ng/internal/api/server.go +++ b/chfs-ng/internal/api/server.go @@ -531,6 +531,9 @@ func (s *Server) upload(w http.ResponseWriter, r *http.Request) { writeJSONError(w, http.StatusForbidden, "has no privilege") return } + // Preflight the complete multipart batch before creating anything. + // This prevents duplicate/bad entries from leaving a partial upload. + seen := make(map[string]struct{}) var saved []string for _, hs := range r.MultipartForm.File { for _, h := range hs { @@ -540,6 +543,11 @@ func (s *Server) upload(w http.ResponseWriter, r *http.Request) { return } target := path.Join(dir, name) + if _, ok := seen[target]; ok { + writeJSONError(w, http.StatusBadRequest, "invalid parameter") + return + } + seen[target] = struct{}{} if _, e := s.Store.Stat(target); e == nil { writeJSONError(w, http.StatusBadRequest, "invalid parameter") return @@ -547,6 +555,12 @@ func (s *Server) upload(w http.ResponseWriter, r *http.Request) { http.Error(w, e.Error(), 500) return } + // Writing happens only after every part has passed validation. + } + } + for _, hs := range r.MultipartForm.File { + for _, h := range hs { + target := path.Join(dir, filepath.ToSlash(h.Filename)) if e := savePart(s.Store, target, h); e != nil { http.Error(w, e.Error(), 500) return @@ -590,7 +604,9 @@ func (s *Server) upload2(w http.ResponseWriter, r *http.Request) { http.Error(w, err.Error(), 400) return } - var saved []string + // Preflight the complete multipart batch before creating anything. + // This prevents duplicate/bad entries from leaving a partial upload. + seen := make(map[string]struct{}) for _, hs := range r.MultipartForm.File { for _, h := range hs { name := filepath.ToSlash(h.Filename) @@ -600,6 +616,11 @@ func (s *Server) upload2(w http.ResponseWriter, r *http.Request) { return } target := path.Join(dir, name) + if _, ok := seen[target]; ok { + writeJSONError(w, http.StatusBadRequest, "invalid parameter") + return + } + seen[target] = struct{}{} if _, e := s.Store.Stat(target); e == nil { writeJSONError(w, http.StatusBadRequest, "invalid parameter") return @@ -607,11 +628,16 @@ func (s *Server) upload2(w http.ResponseWriter, r *http.Request) { http.Error(w, e.Error(), 500) return } + } + } + for _, hs := range r.MultipartForm.File { + for _, h := range hs { + name := strings.TrimPrefix(filepath.ToSlash(h.Filename), "/") + target := path.Join(dir, name) if err := savePart(s.Store, target, h); err != nil { http.Error(w, err.Error(), 500) return } - saved = append(saved, path.Join(dir, name)) } } writeCreated(w) diff --git a/chfs-ng/internal/api/upload_compat_test.go b/chfs-ng/internal/api/upload_compat_test.go new file mode 100644 index 0000000..cee4549 --- /dev/null +++ b/chfs-ng/internal/api/upload_compat_test.go @@ -0,0 +1,156 @@ +package api + +import ( + "bytes" + "mime/multipart" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "testing" + + "chfs-ng/internal/auth" + "chfs-ng/internal/storage" +) + +func uploadCompatServer(t *testing.T) (*Server, string) { + t.Helper() + root := t.TempDir() + if err := os.MkdirAll(filepath.Join(root, "dir"), 0o755); err != nil { + t.Fatal(err) + } + return &Server{Store: storage.Local{Root: root}, Auth: &auth.Manager{Accounts: map[string]*auth.Account{}, Guest: &auth.Account{Mask: auth.Read | auth.Write | auth.Delete}}, RemoveMode: 3}, root +} + +func makeUploadRequest(t *testing.T, method, target string, fieldName, fieldValue, fileName, content string) (*httptest.ResponseRecorder, *http.Request) { + t.Helper() + var b bytes.Buffer + mw := multipart.NewWriter(&b) + if fieldName != "" { + if err := mw.WriteField(fieldName, fieldValue); err != nil { + t.Fatal(err) + } + } + if fileName != "" { + fw, err := mw.CreateFormFile("file", fileName) + if err != nil { + t.Fatal(err) + } + if _, err := fw.Write([]byte(content)); err != nil { + t.Fatal(err) + } + } + if err := mw.Close(); err != nil { + t.Fatal(err) + } + req := httptest.NewRequest(method, target, &b) + req.Header.Set("Content-Type", mw.FormDataContentType()) + return httptest.NewRecorder(), req +} + +func TestUploadNormalizesClientFilenamesInsideTargetFolder(t *testing.T) { + s, root := uploadCompatServer(t) + for _, tc := range []struct{ input, output string }{{"../escape1.txt", "escape1.txt"}, {"/absolute3.txt", "absolute3.txt"}} { + rr, req := makeUploadRequest(t, http.MethodPost, "/chfs/upload?folder=dir", "", "", tc.input, "x") + s.Handler().ServeHTTP(rr, req) + if rr.Code != http.StatusCreated { + t.Errorf("upload %q: status=%d", tc.input, rr.Code) + } + if _, err := os.Stat(filepath.Join(root, tc.output)); !os.IsNotExist(err) { + t.Fatalf("filename escaped target folder: %v", err) + } + if _, err := os.Stat(filepath.Join(root, "dir", tc.output)); err != nil { + t.Fatalf("normalized upload %s missing: %v", tc.output, err) + } + } +} + +func TestUpload2UsesQueryFolderAndRejectsTraversal(t *testing.T) { + s, root := uploadCompatServer(t) + rr, req := makeUploadRequest(t, http.MethodPost, "/chfs/upload2?folder=dir", "", "", "nested.txt", "hello") + s.Handler().ServeHTTP(rr, req) + if rr.Code != http.StatusCreated { + t.Fatalf("upload2 status=%d body=%s", rr.Code, rr.Body.String()) + } + got, err := os.ReadFile(filepath.Join(root, "dir", "nested.txt")) + if err != nil { + t.Fatal(err) + } + if string(got) != "hello" { + t.Fatalf("content=%q", got) + } + for _, folder := range []string{"../", "..%2F", "..\\\\"} { + rr, req = makeUploadRequest(t, http.MethodPost, "/chfs/upload2?folder="+folder, "", "", "bad.txt", "x") + s.Handler().ServeHTTP(rr, req) + if rr.Code != http.StatusBadRequest { + t.Errorf("upload2 folder %q: status=%d", folder, rr.Code) + } + } +} +func TestUploadRejectsDuplicateWithoutOverwriting(t *testing.T) { + s, root := uploadCompatServer(t) + target := filepath.Join(root, "dir", "same.txt") + if err := os.WriteFile(target, []byte("original"), 0o644); err != nil { + t.Fatal(err) + } + rr, req := makeUploadRequest(t, http.MethodPost, "/chfs/upload", "folder", "dir", "same.txt", "replacement") + s.Handler().ServeHTTP(rr, req) + if rr.Code != http.StatusBadRequest { + t.Fatalf("status=%d", rr.Code) + } + got, err := os.ReadFile(target) + if err != nil { + t.Fatal(err) + } + if string(got) != "original" { + t.Fatalf("duplicate upload overwrote file: %q", got) + } +} + +func TestUpload2RejectsDuplicateWithoutOverwriting(t *testing.T) { + s, root := uploadCompatServer(t) + target := filepath.Join(root, "dir", "same.txt") + if err := os.WriteFile(target, []byte("original"), 0o644); err != nil { + t.Fatal(err) + } + rr, req := makeUploadRequest(t, http.MethodPost, "/chfs/upload2?folder=dir", "", "", "same.txt", "replacement") + s.Handler().ServeHTTP(rr, req) + if rr.Code != http.StatusBadRequest { + t.Fatalf("status=%d", rr.Code) + } + got, err := os.ReadFile(target) + if err != nil { + t.Fatal(err) + } + if string(got) != "original" { + t.Fatalf("duplicate upload2 overwrote file: %q", got) + } +} + +func TestUpload2BatchWritesAllFiles(t *testing.T) { + s, root := uploadCompatServer(t) + var b bytes.Buffer + mw := multipart.NewWriter(&b) + for _, name := range []string{"first.txt", "second.txt"} { + fw, err := mw.CreateFormFile("file", name) + if err != nil { + t.Fatal(err) + } + _, _ = fw.Write([]byte(name)) + } + if err := mw.Close(); err != nil { + t.Fatal(err) + } + req := httptest.NewRequest(http.MethodPost, "/chfs/upload2?folder=dir", &b) + req.Header.Set("Content-Type", mw.FormDataContentType()) + rr := httptest.NewRecorder() + s.Handler().ServeHTTP(rr, req) + if rr.Code != http.StatusCreated { + t.Fatalf("status=%d body=%s", rr.Code, rr.Body.String()) + } + for _, name := range []string{"first.txt", "second.txt"} { + if _, err := os.Stat(filepath.Join(root, "dir", name)); err != nil { + t.Fatalf("%s missing: %v", name, err) + } + } +}