admin: keep read-only sessions out of the filer /etc tree (#12057)

* admin: keep read-only sessions out of the filer /etc tree

The file browser read routes are open to any authenticated session, so a
read-only login could download or view arbitrary filer paths. The filer
stores the credential and cluster configuration under /etc (identity JSON
with plaintext S3 secret keys, SSE-S3 keys, remote storage config), which
let a view-only session recover live secrets that the user API redacts.

Gate the read paths on the session role: read-only sessions get 403 for
/api/files/download, /view, /properties, /metadata on /etc paths, the
browser page redirects away, and recursive metadata exports skip the /etc
subtree for read-only sessions.

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>

* admin: only reject real .. path components in file browser paths

path.Clean already resolves interior parent references, so any '..'
surviving in the cleaned path can only be a leading escape from a
relative input or part of a valid name. Matching on the substring
rejected legitimate entries like /buckets/reports/report..csv.

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>

* admin: cover the read-only /etc export exclusion with a test

Pull the excluded-prefix selection into exportExcludedPrefixes so the
role-dependent /etc skip is exercised directly — the cleanReadPath
tests alone would still pass if the exclusion were dropped.

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>

---------

Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>

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

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) {
 		}
 	}
 }
-