fix(search): apply access filtering before paginating results

* fix: replace strings.HasPrefix with utils.IsSubPath for path validation

Signed-off-by: MadDogOwner <[email protected]>

* fix: re-validate shared paths to ensure they remain within the creator's base path

Signed-off-by: MadDogOwner <[email protected]>

---------

Signed-off-by: MadDogOwner <[email protected]>
Co-authored-by: MadDogOwner <[email protected]>

* fix(search): apply access filtering before paginating results

Co-authored-by: Codex <[email protected]>
Signed-off-by: MadDogOwner <[email protected]>

---------

Signed-off-by: MadDogOwner <[email protected]>
Signed-off-by: jyxjjj <[email protected]>
Co-authored-by: MadDogOwner <[email protected]>
Co-authored-by: Codex <[email protected]>

Author: 3 people

internal/search/bleve/search.go

@@ -26,18 +26,9 @@ func (b *Bleve) Config() searcher.Config {
 }
 
 func (b *Bleve) Search(ctx context.Context, req model.SearchReq) ([]model.SearchNode, int64, error) {
-	var queries []query2.Query
-	query := bleve.NewMatchQuery(req.Keywords)
-	query.SetField("name")
-	queries = append(queries, query)
-	if req.Scope != 0 {
-		isDir := req.Scope == 1
-		isDirQuery := bleve.NewBoolFieldQuery(isDir)
-		queries = append(queries, isDirQuery)
-	}
-	reqQuery := bleve.NewConjunctionQuery(queries...)
+	reqQuery := buildQuery(req)
 	search := bleve.NewSearchRequest(reqQuery)
-	search.SortBy([]string{"name"})
+	search.SortBy([]string{"name", "_id"})
 	search.From = (req.Page - 1) * req.PerPage
 	search.Size = req.PerPage
 	search.Fields = []string{"*"}
@@ -47,14 +38,74 @@ func (b *Bleve) Search(ctx context.Context, req model.SearchReq) ([]model.Search
 		return nil, 0, err
 	}
 	res, err := utils.SliceConvert(searchResults.Hits, func(src *search2.DocumentMatch) (model.SearchNode, error) {
-		return model.SearchNode{
-			Parent: src.Fields["parent"].(string),
-			Name:   src.Fields["name"].(string),
-			IsDir:  src.Fields["is_dir"].(bool),
-			Size:   int64(src.Fields["size"].(float64)),
-		}, nil
+		return searchNodeFromHit(src), nil
 	})
-	return res, int64(searchResults.Total), nil
+	return res, int64(searchResults.Total), err
+}
+
+const searchBatchSize = 1000
+
+func (b *Bleve) SearchFiltered(ctx context.Context, req model.SearchReq, filter searcher.Filter) ([]model.SearchNode, int64, error) {
+	reqQuery := buildQuery(req)
+	from := int64(req.Page-1) * int64(req.PerPage)
+	to := from + int64(req.PerPage)
+	var (
+		result      []model.SearchNode
+		total       int64
+		searchAfter []string
+	)
+	for {
+		search := bleve.NewSearchRequest(reqQuery)
+		search.SortBy([]string{"name", "_id"})
+		search.Size = searchBatchSize
+		search.Fields = []string{"*"}
+		if searchAfter != nil {
+			search.SetSearchAfter(searchAfter)
+		}
+		searchResults, err := b.BIndex.Search(search)
+		if err != nil {
+			log.Errorf("search error: %+v", err)
+			return nil, 0, err
+		}
+		for _, hit := range searchResults.Hits {
+			node := searchNodeFromHit(hit)
+			if !utils.IsSubPath(req.Parent, node.Parent) || filter != nil && !filter(node) {
+				continue
+			}
+			if total >= from && total < to {
+				result = append(result, node)
+			}
+			total++
+		}
+		if len(searchResults.Hits) < searchBatchSize {
+			break
+		}
+		last := searchResults.Hits[len(searchResults.Hits)-1]
+		searchAfter = append(searchAfter[:0], last.Sort...)
+	}
+	return result, total, nil
+}
+
+func buildQuery(req model.SearchReq) query2.Query {
+	var queries []query2.Query
+	query := bleve.NewMatchQuery(req.Keywords)
+	query.SetField("name")
+	queries = append(queries, query)
+	if req.Scope != 0 {
+		isDir := req.Scope == 1
+		isDirQuery := bleve.NewBoolFieldQuery(isDir)
+		queries = append(queries, isDirQuery)
+	}
+	return bleve.NewConjunctionQuery(queries...)
+}
+
+func searchNodeFromHit(src *search2.DocumentMatch) model.SearchNode {
+	return model.SearchNode{
+		Parent: src.Fields["parent"].(string),
+		Name:   src.Fields["name"].(string),
+		IsDir:  src.Fields["is_dir"].(bool),
+		Size:   int64(src.Fields["size"].(float64)),
+	}
 }
 
 func (b *Bleve) Index(ctx context.Context, node model.SearchNode) error {
@@ -103,3 +154,4 @@ func (b *Bleve) Clear(ctx context.Context) error {
 }
 
 var _ searcher.Searcher = (*Bleve)(nil)
+var _ searcher.FilteredSearcher = (*Bleve)(nil)

internal/search/bleve/search_test.go

@@ -0,0 +1,52 @@
+package bleve
+
+import (
+	"context"
+	"fmt"
+	"testing"
+
+	"github.com/OpenListTeam/OpenList/v4/internal/model"
+	blevelib "github.com/blevesearch/bleve/v2"
+)
+
+func TestSearchFilteredKeepsDuplicateSortValuesAcrossBatches(t *testing.T) {
+	indexMapping := blevelib.NewIndexMapping()
+	searchNodeMapping := blevelib.NewDocumentMapping()
+	searchNodeMapping.AddFieldMappingsAt("is_dir", blevelib.NewBooleanFieldMapping())
+	searchNodeMapping.AddFieldMappingsAt("parent", blevelib.NewTextFieldMapping())
+	searchNodeMapping.AddFieldMappingsAt("name", blevelib.NewKeywordFieldMapping())
+	indexMapping.AddDocumentMapping("SearchNode", searchNodeMapping)
+	index, err := blevelib.NewMemOnly(indexMapping)
+	if err != nil {
+		t.Fatalf("NewMemOnly() error = %v", err)
+	}
+	t.Cleanup(func() { _ = index.Close() })
+
+	batch := index.NewBatch()
+	for i := 0; i < searchBatchSize+1; i++ {
+		batch.Index(fmt.Sprintf("allowed-%04d", i), model.SearchNode{
+			Parent: "/base",
+			Name:   "duplicate",
+		})
+	}
+	batch.Index("denied", model.SearchNode{Parent: "/base2", Name: "duplicate"})
+	if err := index.Batch(batch); err != nil {
+		t.Fatalf("Batch() error = %v", err)
+	}
+
+	b := &Bleve{BIndex: index}
+	nodes, total, err := b.SearchFiltered(context.Background(), model.SearchReq{
+		Parent:   "/base",
+		Keywords: "duplicate",
+		PageReq:  model.PageReq{Page: 1, PerPage: searchBatchSize + 1},
+	}, nil)
+	if err != nil {
+		t.Fatalf("SearchFiltered() error = %v", err)
+	}
+	if total != searchBatchSize+1 {
+		t.Fatalf("SearchFiltered() total = %d, want %d", total, searchBatchSize+1)
+	}
+	if len(nodes) != searchBatchSize+1 {
+		t.Fatalf("SearchFiltered() returned %d nodes, want %d", len(nodes), searchBatchSize+1)
+	}
+}

internal/search/search.go

@@ -51,6 +51,46 @@ func Search(ctx context.Context, req model.SearchReq) ([]model.SearchNode, int64
 	return instance.Search(ctx, req)
 }
 
+const searchBatchSize = 1000
+
+func SearchFiltered(ctx context.Context, req model.SearchReq, filter searcher.Filter) ([]model.SearchNode, int64, error) {
+	if filteredSearcher, ok := instance.(searcher.FilteredSearcher); ok {
+		return filteredSearcher.SearchFiltered(ctx, req, filter)
+	}
+
+	batchReq := req
+	batchReq.Page = 1
+	batchReq.PerPage = searchBatchSize
+	from := int64(req.Page-1) * int64(req.PerPage)
+	to := from + int64(req.PerPage)
+	var (
+		result         []model.SearchNode
+		filteredTotal  int64
+		processedTotal int64
+	)
+	for {
+		nodes, total, err := instance.Search(ctx, batchReq)
+		if err != nil {
+			return nil, 0, err
+		}
+		for _, node := range nodes {
+			if filter != nil && !filter(node) {
+				continue
+			}
+			if filteredTotal >= from && filteredTotal < to {
+				result = append(result, node)
+			}
+			filteredTotal++
+		}
+		processedTotal += int64(len(nodes))
+		if len(nodes) == 0 || processedTotal >= total {
+			break
+		}
+		batchReq.Page++
+	}
+	return result, filteredTotal, nil
+}
+
 func Index(ctx context.Context, parent string, obj model.Obj) error {
 	if instance == nil {
 		return errs.SearchNotAvailable

internal/search/search_test.go

@@ -0,0 +1,61 @@
+package search
+
+import (
+	"context"
+	"testing"
+
+	"github.com/OpenListTeam/OpenList/v4/internal/model"
+	"github.com/OpenListTeam/OpenList/v4/internal/search/searcher"
+)
+
+type filteredSearchStub struct {
+	nodes []model.SearchNode
+}
+
+func (s *filteredSearchStub) Config() searcher.Config {
+	return searcher.Config{Name: "stub"}
+}
+
+func (s *filteredSearchStub) Search(_ context.Context, req model.SearchReq) ([]model.SearchNode, int64, error) {
+	from := (req.Page - 1) * req.PerPage
+	if from >= len(s.nodes) {
+		return nil, int64(len(s.nodes)), nil
+	}
+	to := min(from+req.PerPage, len(s.nodes))
+	return s.nodes[from:to], int64(len(s.nodes)), nil
+}
+
+func (s *filteredSearchStub) Index(context.Context, model.SearchNode) error        { return nil }
+func (s *filteredSearchStub) BatchIndex(context.Context, []model.SearchNode) error { return nil }
+func (s *filteredSearchStub) Get(context.Context, string) ([]model.SearchNode, error) {
+	return nil, nil
+}
+func (s *filteredSearchStub) Del(context.Context, string) error { return nil }
+func (s *filteredSearchStub) Release(context.Context) error     { return nil }
+func (s *filteredSearchStub) Clear(context.Context) error       { return nil }
+
+func TestSearchFilteredFiltersBeforePagination(t *testing.T) {
+	previous := instance
+	instance = &filteredSearchStub{nodes: []model.SearchNode{
+		{Name: "denied-1"},
+		{Name: "allowed-1"},
+		{Name: "denied-2"},
+		{Name: "allowed-2"},
+	}}
+	t.Cleanup(func() { instance = previous })
+
+	nodes, total, err := SearchFiltered(context.Background(), model.SearchReq{
+		PageReq: model.PageReq{Page: 2, PerPage: 1},
+	}, func(node model.SearchNode) bool {
+		return node.Name == "allowed-1" || node.Name == "allowed-2"
+	})
+	if err != nil {
+		t.Fatalf("SearchFiltered() error = %v", err)
+	}
+	if total != 2 {
+		t.Fatalf("SearchFiltered() total = %d, want 2", total)
+	}
+	if len(nodes) != 1 || nodes[0].Name != "allowed-2" {
+		t.Fatalf("SearchFiltered() nodes = %#v, want allowed-2", nodes)
+	}
+}

internal/search/searcher/searcher.go

@@ -11,6 +11,13 @@ type Config struct {
 	AutoUpdate bool
 }
 
+type Filter func(node model.SearchNode) bool
+
+// FilteredSearcher applies filtering before calculating pagination and totals.
+type FilteredSearcher interface {
+	SearchFiltered(ctx context.Context, req model.SearchReq, filter Filter) ([]model.SearchNode, int64, error)
+}
+
 type Searcher interface {
 	// Config of the searcher
 	Config() Config

server/handles/search.go

@@ -43,27 +43,22 @@ func Search(c *gin.Context) {
 		common.ErrorResp(c, err, 400)
 		return
 	}
-	nodes, total, err := search.Search(c, req.SearchReq)
-	if err != nil {
-		common.ErrorResp(c, err, 500)
-		return
-	}
-	var filteredNodes []model.SearchNode
-	for _, node := range nodes {
+	nodes, total, err := search.SearchFiltered(c, req.SearchReq, func(node model.SearchNode) bool {
 		if !utils.IsSubPath(user.BasePath, node.Parent) {
-			continue
+			return false
 		}
 		meta, err := op.GetNearestMeta(node.Parent)
 		if err != nil && !errors.Is(errors.Cause(err), errs.MetaNotFound) {
-			continue
-		}
-		if !common.CanAccess(user, meta, path.Join(node.Parent, node.Name), req.Password) {
-			continue
+			return false
 		}
-		filteredNodes = append(filteredNodes, node)
+		return common.CanAccess(user, meta, path.Join(node.Parent, node.Name), req.Password)
+	})
+	if err != nil {
+		common.ErrorResp(c, err, 500)
+		return
 	}
 	common.SuccessResp(c, common.PageResp{
-		Content: utils.MustSliceConvert(filteredNodes, nodeToSearchResp),
+		Content: utils.MustSliceConvert(nodes, nodeToSearchResp),
 		Total:   total,
 	})
 }