diff --git a/weed/admin/handlers/file_browser_handlers.go b/weed/admin/handlers/file_browser_handlers.go index 15d7a24b03..085679b768 100644 --- a/weed/admin/handlers/file_browser_handlers.go +++ b/weed/admin/handlers/file_browser_handlers.go @@ -11,6 +11,7 @@ import ( "net/http" "os" "path" + "slices" "strconv" "strings" "time" @@ -50,9 +51,14 @@ func NewFileBrowserHandlers(adminServer *dash.AdminServer) *FileBrowserHandlers // ShowFileBrowser renders the file browser page func (h *FileBrowserHandlers) ShowFileBrowser(w http.ResponseWriter, r *http.Request) { // Get path from query parameter, default to root - path := defaultQuery(r.URL.Query().Get("path"), "/") - // Normalize Windows-style paths for consistency - path = util.CleanWindowsPath(path) + dirPath := path.Clean(util.CleanWindowsPath(defaultQuery(r.URL.Query().Get("path"), "/"))) + if !strings.HasPrefix(dirPath, "/") { + dirPath = "/" + dirPath + } + if dash.IsReadOnlyRole(dash.RoleFromContext(r.Context())) && restrictedBrowserPath(dirPath) { + http.Redirect(w, r, dash.P(r.Context(), "/admin?error=Insufficient permissions"), http.StatusSeeOther) + return + } // Get pagination parameters lastFileName := r.URL.Query().Get("lastFileName") @@ -66,7 +72,7 @@ func (h *FileBrowserHandlers) ShowFileBrowser(w http.ResponseWriter, r *http.Req } // Get file browser data with cursor-based pagination - browserData, err := h.adminServer.GetFileBrowser(path, "", lastFileName, pageSize) + browserData, err := h.adminServer.GetFileBrowser(dirPath, "", lastFileName, pageSize) if err != nil { writeJSONError(w, http.StatusInternalServerError, "Failed to get file browser data: "+err.Error()) return @@ -356,14 +362,46 @@ func (h *FileBrowserHandlers) validateAndCleanFilePath(filePath string) (string, cleanPath = "/" + cleanPath } - // Prevent path traversal attacks - if strings.Contains(cleanPath, "..") { + // Reject parent-directory components; names merely containing ".." are valid + if slices.Contains(strings.Split(cleanPath, "/"), "..") { return "", fmt.Errorf("path traversal not allowed") } return cleanPath, nil } +// restrictedBrowserPath reports whether a cleaned path lies inside the filer's +// /etc tree, which holds the credential store and cluster configuration. +func restrictedBrowserPath(cleanPath string) bool { + return cleanPath == strings.TrimSuffix(filer.DirectoryEtcRoot, "/") || + strings.HasPrefix(cleanPath, filer.DirectoryEtcRoot) +} + +// cleanReadPath validates the request's path parameter; read-only sessions are +// denied the /etc tree so the file browser cannot serve stored credentials. +func (h *FileBrowserHandlers) cleanReadPath(w http.ResponseWriter, r *http.Request) (string, bool) { + cleanPath, err := h.validateAndCleanFilePath(r.URL.Query().Get("path")) + if err != nil { + writeJSONError(w, http.StatusBadRequest, err.Error()) + return "", false + } + if dash.IsReadOnlyRole(dash.RoleFromContext(r.Context())) && restrictedBrowserPath(cleanPath) { + writeJSONError(w, http.StatusForbidden, "Access to this path requires admin access") + return "", false + } + return cleanPath, true +} + +// exportExcludedPrefixes lists the subtrees a metadata export skips; read-only +// sessions also lose /etc so stored credentials stay out of the dump. +func exportExcludedPrefixes(r *http.Request) []string { + prefixes := []string{filer.SystemLogDir} + if dash.IsReadOnlyRole(dash.RoleFromContext(r.Context())) { + prefixes = append(prefixes, strings.TrimSuffix(filer.DirectoryEtcRoot, "/")) + } + return prefixes +} + // fetchFileContent fetches file content via the filer gRPC service. It is // used for the "view as text" path, so the maxBytes cap matches the 1 MB // limit the caller already applies before invoking us. @@ -377,9 +415,8 @@ func (h *FileBrowserHandlers) fetchFileContent(filePath string, timeout time.Dur // gRPC service, so the admin file browser keeps working even when the filer // is started with -disableHttp=true. func (h *FileBrowserHandlers) DownloadFile(w http.ResponseWriter, r *http.Request) { - filePath := r.URL.Query().Get("path") - if filePath == "" { - writeJSONError(w, http.StatusBadRequest, "File path is required") + filePath, ok := h.cleanReadPath(w, r) + if !ok { return } inline := r.URL.Query().Get("inline") == "true" @@ -419,9 +456,8 @@ func (t *responseWriteTracker) Write(p []byte) (int, error) { // ViewFile handles file viewing requests (for text files, images, etc.) func (h *FileBrowserHandlers) ViewFile(w http.ResponseWriter, r *http.Request) { - filePath := r.URL.Query().Get("path") - if filePath == "" { - writeJSONError(w, http.StatusBadRequest, "File path is required") + filePath, ok := h.cleanReadPath(w, r) + if !ok { return } @@ -524,9 +560,8 @@ func (h *FileBrowserHandlers) ViewFile(w http.ResponseWriter, r *http.Request) { // GetFileProperties handles file properties requests func (h *FileBrowserHandlers) GetFileProperties(w http.ResponseWriter, r *http.Request) { - filePath := r.URL.Query().Get("path") - if filePath == "" { - writeJSONError(w, http.StatusBadRequest, "File path is required") + filePath, ok := h.cleanReadPath(w, r) + if !ok { return } @@ -630,23 +665,18 @@ func (h *FileBrowserHandlers) GetFileProperties(w http.ResponseWriter, r *http.R // FullEntry stream — the format weed shell fs.meta.load reads. Directories are // walked recursively via the filer BFS metadata stream. func (h *FileBrowserHandlers) ExportMetadata(w http.ResponseWriter, r *http.Request) { - filePath := r.URL.Query().Get("path") - if filePath == "" { - writeJSONError(w, http.StatusBadRequest, "File path is required") - return - } - cleanPath, err := h.validateAndCleanFilePath(filePath) - if err != nil { - writeJSONError(w, http.StatusBadRequest, err.Error()) + cleanPath, ok := h.cleanReadPath(w, r) + if !ok { return } + excludedPrefixes := exportExcludedPrefixes(r) tracker := &responseWriteTracker{ResponseWriter: w} - err = h.adminServer.WithFilerClient(func(client filer_pb.SeaweedFilerClient) error { + err := h.adminServer.WithFilerClient(func(client filer_pb.SeaweedFilerClient) error { stream, err := client.TraverseBfsMetadata(r.Context(), &filer_pb.TraverseBfsMetadataRequest{ Directory: cleanPath, - ExcludedPrefixes: []string{filer.SystemLogDir}, + ExcludedPrefixes: excludedPrefixes, }) if err != nil { return err diff --git a/weed/admin/handlers/file_browser_handlers_test.go b/weed/admin/handlers/file_browser_handlers_test.go index 65d05dad9b..f02fead273 100644 --- a/weed/admin/handlers/file_browser_handlers_test.go +++ b/weed/admin/handlers/file_browser_handlers_test.go @@ -3,9 +3,12 @@ package handlers import ( "net/http" "net/http/httptest" + "slices" + "strings" "testing" "github.com/seaweedfs/seaweedfs/weed/admin/dash" + "github.com/seaweedfs/seaweedfs/weed/filer" ) func TestValidateAndCleanFilePath_AllowsControlChars(t *testing.T) { @@ -26,6 +29,9 @@ func TestValidateAndCleanFilePath_AllowsControlChars(t *testing.T) { {"relative/path.txt", "/relative/path.txt"}, // Duplicate slashes should be collapsed by path.Clean. {"/a//b", "/a/b"}, + // ".." inside a file or directory name is not a parent component. + {"/buckets/reports/report..csv", "/buckets/reports/report..csv"}, + {"/a../b", "/a../b"}, } for _, tc := range cases { got, err := h.validateAndCleanFilePath(tc.in) @@ -46,6 +52,81 @@ func TestValidateAndCleanFilePath_RejectsEmpty(t *testing.T) { } } +func TestValidateAndCleanFilePath_RejectsParentComponents(t *testing.T) { + h := &FileBrowserHandlers{} + for _, p := range []string{"../etc", "../../etc/iam", "..", "a/../../b"} { + if _, err := h.validateAndCleanFilePath(p); err == nil { + t.Errorf("validateAndCleanFilePath(%q): expected traversal rejection", p) + } + } +} + +func TestCleanReadPath_DeniesEtcForReadOnly(t *testing.T) { + h := &FileBrowserHandlers{} + + denied := []string{"/etc", "/etc/iam", "/etc/iam/identities/root.json", + "/etc/s3/kek.json", "/etc/../etc/iam/identity.json", "/a/../../etc/x"} + for _, p := range denied { + req := httptest.NewRequest(http.MethodGet, "/api/files/download?path="+p, nil) + req = req.WithContext(dash.WithAuthContext(req.Context(), "ro", dash.RoleReadOnly, "csrf")) + w := httptest.NewRecorder() + + if _, ok := h.cleanReadPath(w, req); ok { + t.Errorf("path %q: expected denial for read-only session", p) + continue + } + if w.Code != http.StatusForbidden { + t.Errorf("path %q: expected 403, got %d", p, w.Code) + } + } +} + +func TestCleanReadPath_AllowsEtcForAdminAndNorRole(t *testing.T) { + h := &FileBrowserHandlers{} + + for _, role := range []string{"admin", ""} { + req := httptest.NewRequest(http.MethodGet, "/api/files/download?path=/etc/iam/identities/root.json", nil) + req = req.WithContext(dash.WithAuthContext(req.Context(), "u", role, "csrf")) + w := httptest.NewRecorder() + + if _, ok := h.cleanReadPath(w, req); !ok { + t.Errorf("role %q: expected /etc path to be allowed, got %d", role, w.Code) + } + } +} + +func TestCleanReadPath_AllowsReadOnlyOutsideEtc(t *testing.T) { + h := &FileBrowserHandlers{} + + for _, p := range []string{"/buckets/foo/bar.txt", "/etcx/file", "/etcetera", "/"} { + req := httptest.NewRequest(http.MethodGet, "/api/files/download?path="+p, nil) + req = req.WithContext(dash.WithAuthContext(req.Context(), "ro", dash.RoleReadOnly, "csrf")) + w := httptest.NewRecorder() + + if _, ok := h.cleanReadPath(w, req); !ok { + t.Errorf("path %q: expected allowed for read-only session, got %d", p, w.Code) + } + } +} + +func TestExportExcludedPrefixes_ReadOnlyDropsEtc(t *testing.T) { + etcPrefix := strings.TrimSuffix(filer.DirectoryEtcRoot, "/") + + req := httptest.NewRequest(http.MethodGet, "/api/files/metadata/export?path=/", nil) + req = req.WithContext(dash.WithAuthContext(req.Context(), "ro", dash.RoleReadOnly, "csrf")) + if got := exportExcludedPrefixes(req); !slices.Contains(got, etcPrefix) { + t.Errorf("read-only export should exclude %q, got %v", etcPrefix, got) + } + + for _, role := range []string{"admin", ""} { + req := httptest.NewRequest(http.MethodGet, "/api/files/metadata/export?path=/", nil) + req = req.WithContext(dash.WithAuthContext(req.Context(), "u", role, "csrf")) + if got := exportExcludedPrefixes(req); slices.Contains(got, etcPrefix) { + t.Errorf("role %q: export should keep %q, got %v", role, etcPrefix, got) + } + } +} + func TestListFolders_RejectsPathOutsideBuckets(t *testing.T) { h := &FileBrowserHandlers{adminServer: &dash.AdminServer{}} @@ -62,4 +143,3 @@ func TestListFolders_RejectsPathOutsideBuckets(t *testing.T) { } } } -