Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions hub-server/internal/config/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,13 @@ const MaxMessagePageLimit = 100
// MaxMessagePageLimit because sync clients often fetch larger batches.
const MaxIncrementalMessageLimit = 500

// MaxListPageSize is the maximum page size for the generic list endpoints
// (workspaces, skills, agent profiles, execution targets, provider bindings,
// MCP servers, audit events). It is the value api/openapi.yaml declares for the
// shared PageSize parameter, so it is the bound those endpoints must actually
// enforce; it was previously a bare `200` literal repeated in ten places.
const MaxListPageSize = 200

// MaxPageLimit is the maximum allowed page size (pageSize / limit) for list
// endpoints across all Hub handlers. Handlers MUST clamp user-supplied values
// to this ceiling to prevent unbounded query resource usage.
Expand Down
34 changes: 34 additions & 0 deletions hub-server/internal/config/paging.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
package config

// ClampPageSize normalizes a caller-supplied page size into the range the
// endpoint actually enforces:
//
// - requested <= 0 → def (a missing or unparsable pageSize must not mean
// "everything", and must not mean "nothing");
// - requested > max → **max**, not def;
// - otherwise the requested value, unchanged.
//
// The middle rule is the one that was gotten wrong in twelve places (#2154).
// They all read:
//
// if pageSize <= 0 || pageSize > 200 { pageSize = defaultXPageSize }
//
// which turns "you asked for too many" into "here is a quarter of a page", with
// HTTP 200 and no error. Clamping to max keeps the request satisfiable and makes
// the declared contract (api/openapi.yaml: PageSize maximum 200) the bound the
// endpoint really enforces. Falling back to def is only correct for the
// non-positive branch, where there is no request to honour.
//
// max and def are parameters rather than constants because the codebase has two
// legitimate maxima: MaxListPageSize (200) for the generic list endpoints and
// MaxMessagePageLimit (100) / MaxPageLimit (500) for message and document
// families. Callers pass the one their endpoint declares.
func ClampPageSize(requested, max, def int) int {
if requested <= 0 {
return def
}
if requested > max {
return max
}
return requested
}
50 changes: 50 additions & 0 deletions hub-server/internal/config/paging_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
package config

import "testing"

// TestClampPageSize pins the three branches, and in particular that an
// over-maximum request clamps to the maximum instead of collapsing to the
// default — the collapse was the #2154 bug this helper exists to end.
func TestClampPageSize(t *testing.T) {
const (
max = 200
def = 50
)
cases := []struct {
name string
requested int
want int
}{
{"zero falls back to the default", 0, def},
{"negative falls back to the default", -7, def},
{"one is honoured", 1, 1},
{"in-range is honoured exactly", 137, 137},
{"the maximum itself is honoured", max, max},
{"one over the maximum clamps to the maximum", max + 1, max},
{"the handler-layer ceiling clamps to the maximum", MaxPageLimit, max},
{"an absurd value clamps to the maximum", 1 << 20, max},
}
for _, tc := range cases {
tc := tc
t.Run(tc.name, func(t *testing.T) {
if got := ClampPageSize(tc.requested, max, def); got != tc.want {
t.Fatalf("ClampPageSize(%d, %d, %d) = %d, want %d", tc.requested, max, def, got, tc.want)
}
})
}
}

// TestClampPageSize_RespectsCallerMaximum checks the helper is not hardwired to
// one maximum: the message family clamps at MaxMessagePageLimit and the
// document family at MaxPageLimit.
func TestClampPageSize_RespectsCallerMaximum(t *testing.T) {
if got := ClampPageSize(MaxMessagePageLimit+1, MaxMessagePageLimit, DefaultPaginationLimit); got != MaxMessagePageLimit {
t.Fatalf("message-family clamp = %d, want %d", got, MaxMessagePageLimit)
}
if got := ClampPageSize(MaxPageLimit+1, MaxPageLimit, DefaultPaginationLimit); got != MaxPageLimit {
t.Fatalf("document-family clamp = %d, want %d", got, MaxPageLimit)
}
if MaxListPageSize != 200 {
t.Fatalf("MaxListPageSize = %d, want 200: it is the value api/openapi.yaml declares for the shared PageSize parameter, so changing it is a contract change", MaxListPageSize)
}
}
9 changes: 3 additions & 6 deletions hub-server/internal/repository/agent_profile.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"strings"
"time"

"github.com/agenthub/hub-server/internal/config"
"github.com/agenthub/hub-server/internal/errcode"
"github.com/agenthub/hub-server/internal/model"
"gorm.io/gorm"
Expand Down Expand Up @@ -84,9 +85,7 @@ func SoftDeleteAgentProfile(db *gorm.DB, id, ownerID string) error {
// If q is non-empty, filters by name/description ILIKE match.
// If runtimeID is non-empty, filters by runtime_id.
func ListAgentProfiles(db *gorm.DB, ownerID, runtimeID, q, cursor string, pageSize int) ([]model.AgentProfile, bool, error) {
if pageSize <= 0 || pageSize > 200 {
pageSize = defaultProfilePageSize
}
pageSize = config.ClampPageSize(pageSize, config.MaxListPageSize, defaultProfilePageSize)

qry := db.Where("owner_id = ? AND deleted_at IS NULL", ownerID)
if runtimeID != "" {
Expand All @@ -113,9 +112,7 @@ func ListAgentProfiles(db *gorm.DB, ownerID, runtimeID, q, cursor string, pageSi

// ListPublicProfiles returns published profiles for the agent market.
func ListPublicProfiles(db *gorm.DB, runtimeID, q, sortBy, cursor string, pageSize int) ([]model.AgentProfile, bool, error) {
if pageSize <= 0 || pageSize > 200 {
pageSize = defaultProfilePageSize
}
pageSize = config.ClampPageSize(pageSize, config.MaxListPageSize, defaultProfilePageSize)

qry := db.Where("is_public = TRUE AND deleted_at IS NULL")
if runtimeID != "" {
Expand Down
5 changes: 2 additions & 3 deletions hub-server/internal/repository/audit.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package repository
import (
"time"

"github.com/agenthub/hub-server/internal/config"
"github.com/agenthub/hub-server/internal/model"
"gorm.io/gorm"
"gorm.io/gorm/clause"
Expand Down Expand Up @@ -73,9 +74,7 @@ func createAuditEventOnce(db *gorm.DB, event *model.AuditEvent) error {
// When userID is empty, queries across all users (admin). When non-empty, filters to that user.
// Cursor-based pagination uses descending order (newest first): WHERE id < cursor.
func ListAuditEvents(db *gorm.DB, userID, eventType, severity string, since, until *time.Time, cursor string, pageSize int) ([]model.AuditEvent, bool, error) {
if pageSize <= 0 || pageSize > 200 {
pageSize = defaultAuditPageSize
}
pageSize = config.ClampPageSize(pageSize, config.MaxListPageSize, defaultAuditPageSize)

qry := db.Model(&model.AuditEvent{})
if userID != "" {
Expand Down
5 changes: 2 additions & 3 deletions hub-server/internal/repository/execution_target.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package repository
import (
"time"

"github.com/agenthub/hub-server/internal/config"
"github.com/agenthub/hub-server/internal/model"
"gorm.io/gorm"
"gorm.io/gorm/clause"
Expand Down Expand Up @@ -42,9 +43,7 @@ func SoftDeleteExecutionTarget(db *gorm.DB, id, ownerID string) error {
}

func ListExecutionTargets(db *gorm.DB, ownerID, targetType, cursor string, pageSize int) ([]model.ExecutionTarget, bool, error) {
if pageSize <= 0 || pageSize > 200 {
pageSize = defaultTargetPageSize
}
pageSize = config.ClampPageSize(pageSize, config.MaxListPageSize, defaultTargetPageSize)

qry := db.Where("owner_id = ? AND deleted_at IS NULL", ownerID)
if targetType != "" {
Expand Down
9 changes: 3 additions & 6 deletions hub-server/internal/repository/mcp_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package repository
import (
"time"

"github.com/agenthub/hub-server/internal/config"
"github.com/agenthub/hub-server/internal/model"
"gorm.io/gorm"
)
Expand Down Expand Up @@ -38,9 +39,7 @@ func SoftDeleteMCPServer(db *gorm.DB, id, ownerID string) error {

// ListMCPServers returns MCP servers for an owner with optional filters and cursor pagination.
func ListMCPServers(db *gorm.DB, ownerID, q, transport, cursor string, pageSize int) ([]model.MCPServer, bool, error) {
if pageSize <= 0 || pageSize > 200 {
pageSize = defaultMCPServerPageSize
}
pageSize = config.ClampPageSize(pageSize, config.MaxListPageSize, defaultMCPServerPageSize)

qry := db.Where("owner_id = ? AND deleted_at IS NULL", ownerID)
if transport != "" {
Expand All @@ -67,9 +66,7 @@ func ListMCPServers(db *gorm.DB, ownerID, q, transport, cursor string, pageSize

// ListPublicMCPServers returns published MCP servers for the public market.
func ListPublicMCPServers(db *gorm.DB, q, transport, cursor string, pageSize int) ([]model.MCPServer, bool, error) {
if pageSize <= 0 || pageSize > 200 {
pageSize = defaultMCPServerPageSize
}
pageSize = config.ClampPageSize(pageSize, config.MaxListPageSize, defaultMCPServerPageSize)

qry := db.Where("is_public = TRUE AND deleted_at IS NULL")
if transport != "" {
Expand Down
4 changes: 1 addition & 3 deletions hub-server/internal/repository/message.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,7 @@ func GetMessageBySessionAndID(db *gorm.DB, sessionID, id string) (*model.Message
}

func GetMessagesBySession(db *gorm.DB, sessionID string, beforeSeq int64, limit int) ([]model.Message, error) {
if limit <= 0 || limit > config.MaxMessagePageLimit {
limit = config.DefaultPaginationLimit
}
limit = config.ClampPageSize(limit, config.MaxMessagePageLimit, config.DefaultPaginationLimit)
var msgs []model.Message
query := db.Where("session_id = ?", sessionID)
if beforeSeq > 0 {
Expand Down
4 changes: 1 addition & 3 deletions hub-server/internal/repository/notification.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,7 @@ func CreateNotification(db *gorm.DB, n *model.Notification) error {
}

func ListNotifications(db *gorm.DB, userID string, unreadOnly bool, limit, offset int) ([]model.Notification, error) {
if limit <= 0 || limit > config.MaxMessagePageLimit {
limit = config.DefaultPaginationLimit
}
limit = config.ClampPageSize(limit, config.MaxMessagePageLimit, config.DefaultPaginationLimit)
// Defense in depth: clamp offset at the repository choke point too
// (handler already clamps; other callers must not bypass, #2154).
if offset < 0 {
Expand Down
Loading
Loading