Skip to content

Commit 664f7cc

Browse files
committed
fix(server): validate order_by on the sidebar-index route; tighten doc guard
The session-filter input struct is shared by the list and sidebar-index routes. Dropping the order_by enum left the sidebar route accepting malformed specs like ?order_by=bogus (200, silently ignored) where the enum used to reject them. Validate order_by in dbFilter() too so both routes return 400 on bad input. Replace the substring-based doc guards with one test that parses the "Valid keys:" clause into exact tokens and compares to db.SortKeys() in order, so a key omitted from the clause can no longer be masked by the example text.
1 parent 2f87182 commit 664f7cc

3 files changed

Lines changed: 34 additions & 26 deletions

File tree

internal/server/huma_routes_sessions.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,12 @@ func (in *sessionFilterInput) dbFilter(includeChildren bool) (db.SessionFilter,
137137
if err := validateDateFilterValues(in.Date, in.DateFrom, in.DateTo, in.ActiveSince); err != nil {
138138
return db.SessionFilter{}, err
139139
}
140+
// The order_by param is shared with the list route via this struct; reject
141+
// malformed specs here too (the dropped enum used to guard every route),
142+
// even though the sidebar index applies its own ordering and ignores it.
143+
if _, err := db.ParseSortSpec(in.OrderBy); err != nil {
144+
return db.SessionFilter{}, apiError(http.StatusBadRequest, "invalid order_by: "+err.Error())
145+
}
140146
limit := 0
141147
if in.Limit > 0 {
142148
limit = clampLimit(in.Limit, db.DefaultSessionLimit, db.MaxSessionLimit)

internal/server/sort_http_multikey_test.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,3 +103,18 @@ func TestListSessions_OrderBy_Invalid(t *testing.T) {
103103
})
104104
}
105105
}
106+
107+
// TestSidebarIndex_OrderByInvalid confirms the sidebar-index route, which shares
108+
// the session-filter input struct, also rejects a malformed order_by rather than
109+
// silently accepting and ignoring it (the dropped enum used to guard both routes).
110+
func TestSidebarIndex_OrderByInvalid(t *testing.T) {
111+
te := setup(t)
112+
te.seedSession(t, "s1", "p", 3)
113+
114+
w := te.get(t, "/api/v1/sessions/sidebar-index?order_by=bogus")
115+
assertStatus(t, w, http.StatusBadRequest)
116+
117+
// A valid spec is still accepted even though the sidebar applies its own order.
118+
w = te.get(t, "/api/v1/sessions/sidebar-index?order_by=messages:desc")
119+
assertStatus(t, w, http.StatusOK)
120+
}

internal/server/sort_internal_test.go

Lines changed: 13 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -11,35 +11,22 @@ import (
1111
"go.kenn.io/agentsview/internal/db"
1212
)
1313

14-
// TestSortKeysDocumented guards the order_by doc string against drift from the
15-
// shared sort registry: every accepted sort key must be named in the parameter
16-
// documentation so the OpenAPI surface lists the full allow-list (the enum tag
17-
// could no longer express the comma-separated key:dir spec).
18-
func TestSortKeysDocumented(t *testing.T) {
14+
// TestSortKeysMatchDoc guards the order_by doc string against drift from the
15+
// shared sort registry. It parses the "Valid keys:" clause into exact tokens and
16+
// requires them to equal db.SortKeys() in order, so a key omitted from the clause
17+
// cannot be masked by the example text earlier in the doc, and a stale key left
18+
// after a rename is caught too. This replaces the enum-sync guard that the dropped
19+
// order_by enum tag used to provide.
20+
func TestSortKeysMatchDoc(t *testing.T) {
1921
field, ok := reflect.TypeFor[sessionFilterInput]().FieldByName("OrderBy")
2022
require.True(t, ok, "sessionFilterInput.OrderBy field")
2123
doc := field.Tag.Get("doc")
22-
require.NotEmpty(t, doc, "order_by doc tag")
23-
for _, key := range db.SortKeys() {
24-
assert.Contains(t, doc, key, "order_by doc must mention sort key %q", key)
25-
}
26-
}
27-
28-
// TestSortKeysDocOmitsStaleKeys is the inverse guard: the "Valid keys:" clause
29-
// should not name a key that no longer exists in the registry (a stale entry
30-
// left after a rename).
31-
func TestSortKeysDocOmitsStaleKeys(t *testing.T) {
32-
field, _ := reflect.TypeFor[sessionFilterInput]().FieldByName("OrderBy")
33-
doc := field.Tag.Get("doc")
3424
_, after, found := strings.Cut(doc, "Valid keys:")
35-
require.True(t, found, "doc should enumerate valid keys")
36-
valid := make(map[string]bool, len(db.SortKeys()))
37-
for _, k := range db.SortKeys() {
38-
valid[k] = true
39-
}
40-
for _, tok := range strings.FieldsFunc(after, func(r rune) bool {
25+
require.True(t, found, "order_by doc must enumerate keys after 'Valid keys:'")
26+
27+
keys := strings.FieldsFunc(after, func(r rune) bool {
4128
return r == ',' || r == ' ' || r == '.'
42-
}) {
43-
assert.True(t, valid[tok], "doc lists unknown sort key %q", tok)
44-
}
29+
})
30+
assert.Equal(t, db.SortKeys(), keys,
31+
"order_by 'Valid keys:' clause must list exactly db.SortKeys() in order")
4532
}

0 commit comments

Comments
 (0)