Received-SPF: Pass (mailfrom) identity=mailfrom; client-ip=209.85.221.54; helo=mail-wr1-f54.google.com; envelope-from=peter@netlandish.com; receiver=<UNKNOWN> 
From: Peter Sanchez <peter@netlandish.com>
To: ~netlandish/links-dev@lists.code.netlandish.com
Cc: Peter Sanchez <peter@netlandish.com>
Subject: [PATCH links] bugfix: Invalid cursors raise 500 errors.
Date: Tue, 18 Aug 2026 12:56:07 -0600
Message-ID: <20260818185609.1621-1-peter@netlandish.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit

Changelog-fixed: invalid cursors no longer trigger ISE's
---
 admin/routes.go              | 38 ++++-----------
 api/graph/model/cursor.go    | 17 ++++++-
 api/graph/pagination.go      |  7 +++
 api/graph/pagination_test.go | 47 ++++++++++++++++++
 billing/routes.go            |  8 +---
 core/routes.go               | 16 ++-----
 core/routes_test.go          | 53 +++++++++++++++++++++
 helpers.go                   | 28 ++++++++---
 helpers_test.go              | 92 ++++++++++++++++++++++++++++++++++++
 list/routes.go               | 22 ++-------
 short/routes.go              |  8 +---
 11 files changed, 258 insertions(+), 78 deletions(-)
 create mode 100644 helpers_test.go

diff --git a/admin/routes.go b/admin/routes.go
index e36c97e..2f7e7a7 100644
--- a/admin/routes.go
+++ b/admin/routes.go
@@ -449,17 +449,13 @@ func (s *Service) OrgDetail(c echo.Context) error {
 			}
 		}`)
 	op.Var("orgSlug", result.Org.Slug)
-	if c.QueryParam("next") != "" {
-		op.Var("after", c.QueryParam("next"))
-	} else if c.QueryParam("prev") != "" {
-		op.Var("before", c.QueryParam("prev"))
-	}
+	reversed := links.SetCursorVars(c, op)
 	err = links.Execute(c.Request().Context(), op, &historyResult)
 	if err != nil {
 		return err
 	}
 
-	if c.QueryParam("prev") != "" {
+	if reversed {
 		slices.Reverse(historyResult.Payments.Result)
 	}
 
@@ -995,11 +991,7 @@ func (s *Service) BillingList(c echo.Context) error {
 				}
 			}
 		}`)
-	if c.QueryParam("next") != "" {
-		op.Var("after", c.QueryParam("next"))
-	} else if c.QueryParam("prev") != "" {
-		op.Var("before", c.QueryParam("prev"))
-	}
+	reversed := links.SetCursorVars(c, op)
 
 	if sDate != "" && eDate != "" {
 		op.Var("dStart", sDate)
@@ -1012,7 +1004,7 @@ func (s *Service) BillingList(c echo.Context) error {
 		return err
 	}
 
-	if c.QueryParam("prev") != "" {
+	if reversed {
 		slices.Reverse(historyResult.Payments.Result)
 	}
 
@@ -1144,11 +1136,7 @@ func (s *Service) DomainList(c echo.Context) error {
 		gmap["search"] = query
 	}
 
-	if c.QueryParam("next") != "" {
-		op.Var("after", c.QueryParam("next"))
-	} else if c.QueryParam("prev") != "" {
-		op.Var("before", c.QueryParam("prev"))
-	}
+	reversed := links.SetCursorVars(c, op)
 
 	if c.QueryParam("level") != "" {
 		level := c.QueryParam("level")
@@ -1175,7 +1163,7 @@ func (s *Service) DomainList(c echo.Context) error {
 	if err != nil {
 		return err
 	}
-	if c.QueryParam("prev") != "" {
+	if reversed {
 		slices.Reverse(result.Organizations.Result)
 	}
 	if result.Organizations.PageInfo.HasPrevPage {
@@ -1246,11 +1234,7 @@ func (s *Service) OrgList(c echo.Context) error {
 		gmap["search"] = query
 	}
 
-	if c.QueryParam("next") != "" {
-		op.Var("after", c.QueryParam("next"))
-	} else if c.QueryParam("prev") != "" {
-		op.Var("before", c.QueryParam("prev"))
-	}
+	links.SetCursorVars(c, op)
 	err := links.Execute(c.Request().Context(), op, &result)
 	if err != nil {
 		return err
@@ -1635,16 +1619,12 @@ func (s *Service) UserList(c echo.Context) error {
 		op.Var("search", query)
 		gmap["search"] = query
 	}
-	if c.QueryParam("next") != "" {
-		op.Var("after", c.QueryParam("next"))
-	} else if c.QueryParam("prev") != "" {
-		op.Var("before", c.QueryParam("prev"))
-	}
+	reversed := links.SetCursorVars(c, op)
 	err := links.Execute(c.Request().Context(), op, &result)
 	if err != nil {
 		return err
 	}
-	if c.QueryParam("prev") != "" {
+	if reversed {
 		slices.Reverse(result.GetUsers.Result)
 	}
 	if result.GetUsers.PageInfo.HasPrevPage {
diff --git a/api/graph/model/cursor.go b/api/graph/model/cursor.go
index 3e04a20..f4e05eb 100644
--- a/api/graph/model/cursor.go
+++ b/api/graph/model/cursor.go
@@ -5,6 +5,9 @@ import (
 	"encoding/json"
 	"fmt"
 	"io"
+	"links/valid"
+
+	"github.com/vektah/gqlparser/v2/gqlerror"
 )
 
 const (
@@ -20,6 +23,16 @@ type Cursor struct {
 	Limit  int
 }
 
+// errInvalidCursor carries a validation code so consumers running the error
+// through ParseInputErrors render a form error rather than falling through to
+// the raw error path, which the web tier reports as a 500.
+func errInvalidCursor() error {
+	return &gqlerror.Error{
+		Message:    "Invalid cursor",
+		Extensions: map[string]any{"code": valid.ErrValidationGlobalCode},
+	}
+}
+
 func (c *Cursor) UnmarshalGQL(v any) error {
 	enc, ok := v.(string)
 	if !ok {
@@ -27,10 +40,10 @@ func (c *Cursor) UnmarshalGQL(v any) error {
 	}
 	plain, err := base64.StdEncoding.DecodeString(enc)
 	if plain == nil || err != nil {
-		return fmt.Errorf("Invalid cursor")
+		return errInvalidCursor()
 	}
 	if err = json.Unmarshal(plain, c); err != nil {
-		return fmt.Errorf("Invalid cursor")
+		return errInvalidCursor()
 	}
 	return nil
 }
diff --git a/api/graph/pagination.go b/api/graph/pagination.go
index 0c16595..09068e1 100644
--- a/api/graph/pagination.go
+++ b/api/graph/pagination.go
@@ -110,6 +110,13 @@ func QueryModel[T any](
 		numElements = *limit
 	}
 
+	// A client-supplied cursor carries its own Limit; a zero or negative value
+	// would drop the SQL LIMIT clause entirely in GetBuilder and then panic the
+	// slice reslice in PaginateResults.
+	if numElements <= 0 {
+		numElements = model.PaginationDefault
+	}
+
 	// maxLimit will default to model.PaginationMax
 	maxLimit := ForPaginationContext(ctx)
 	if numElements > maxLimit {
diff --git a/api/graph/pagination_test.go b/api/graph/pagination_test.go
index 82e7500..6307936 100644
--- a/api/graph/pagination_test.go
+++ b/api/graph/pagination_test.go
@@ -1,11 +1,13 @@
 package graph
 
 import (
+	"context"
 	"links/api/graph/model"
 	"slices"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"netlandish.com/x/gobwebs/database"
 )
 
 func TestPaginationForwardAndBackward(t *testing.T) {
@@ -75,3 +77,48 @@ func TestPaginationForwardAndBackward(t *testing.T) {
 		assert.Equal(t, 26, len(seen))
 	})
 }
+
+func TestQueryModelClampsCursorLimit(t *testing.T) {
+	items := make([]int, 0, 200)
+	for i := 200; i > 0; i-- {
+		items = append(items, i)
+	}
+
+	run := func(t *testing.T, limit *int, before, after *model.Cursor) int {
+		t.Helper()
+		var gotLimit int
+		getModels := func(_ context.Context, opts *database.FilterOptions) ([]int, error) {
+			gotLimit = opts.Limit
+			if opts.Limit <= 0 || opts.Limit > len(items) {
+				return items, nil
+			}
+			return items[:opts.Limit], nil
+		}
+		_, _, err := QueryModel(context.Background(), &database.FilterOptions{},
+			"id", "DESC", limit, before, after, getModels, func(v int) int { return v })
+		assert.NoError(t, err)
+		return gotLimit
+	}
+
+	// A negative cursor limit used to leave opts.Limit at zero, which drops the SQL
+	// LIMIT clause entirely and then panics on the reslice in PaginateResults.
+	t.Run("negative after limit", func(t *testing.T) {
+		assert.Equal(t, model.PaginationDefault+1,
+			run(t, nil, nil, &model.Cursor{After: 150, Limit: -1}))
+	})
+
+	t.Run("zero before limit", func(t *testing.T) {
+		assert.Equal(t, model.PaginationDefault+1,
+			run(t, nil, &model.Cursor{Before: 150, Limit: 0}, nil))
+	})
+
+	t.Run("negative explicit limit falls back to cursor", func(t *testing.T) {
+		negative := -5
+		assert.Equal(t, 11, run(t, &negative, nil, &model.Cursor{After: 150, Limit: 10}))
+	})
+
+	t.Run("oversized cursor limit still capped", func(t *testing.T) {
+		assert.Equal(t, model.PaginationMax+1,
+			run(t, nil, nil, &model.Cursor{After: 150, Limit: 10000}))
+	})
+}
diff --git a/billing/routes.go b/billing/routes.go
index 82fd992..524ce9e 100644
--- a/billing/routes.go
+++ b/billing/routes.go
@@ -253,17 +253,13 @@ func (s *Service) SubscriptionHistory(c echo.Context) error {
 			}
 		}`)
 	op.Var("orgSlug", slug)
-	if c.QueryParam("next") != "" {
-		op.Var("after", c.QueryParam("next"))
-	} else if c.QueryParam("prev") != "" {
-		op.Var("before", c.QueryParam("prev"))
-	}
+	reversed := links.SetCursorVars(c, op)
 	err = links.Execute(c.Request().Context(), op, &result)
 	if err != nil {
 		return err
 	}
 
-	if c.QueryParam("prev") != "" {
+	if reversed {
 		slices.Reverse(result.Payments.Result)
 	}
 
diff --git a/core/routes.go b/core/routes.go
index 48da02b..2b2f835 100644
--- a/core/routes.go
+++ b/core/routes.go
@@ -1847,11 +1847,7 @@ func (s *Service) UserFeed(c echo.Context) error {
 		}
 	}`)
 
-	if c.QueryParam("next") != "" {
-		op.Var("after", c.QueryParam("next"))
-	} else if c.QueryParam("prev") != "" {
-		op.Var("before", c.QueryParam("prev"))
-	}
+	reversed := links.SetCursorVars(c, op)
 
 	var (
 		tag, excludeTag, search string
@@ -1898,7 +1894,7 @@ func (s *Service) UserFeed(c echo.Context) error {
 	pd.Data["clear"] = lt.Translate("Clear")
 	pd.Data["followings"] = lt.Translate("Followings")
 	orgLinks := result.OrgLinks.Result
-	if c.QueryParam("prev") != "" {
+	if reversed {
 		slices.Reverse(orgLinks)
 	}
 
@@ -2287,11 +2283,7 @@ func (s *Service) OrgLinksList(c echo.Context) error {
 		currURL = c.Echo().Reverse(s.RouteName("recent_link_list"))
 		rssURL = c.Echo().Reverse(s.RouteName("recent_link_list_rss"))
 	}
-	if c.QueryParam("next") != "" {
-		op.Var("after", c.QueryParam("next"))
-	} else if c.QueryParam("prev") != "" {
-		op.Var("before", c.QueryParam("prev"))
-	}
+	reversed := links.SetCursorVars(c, op)
 
 	var (
 		hasUnreadFilter,
@@ -2485,7 +2477,7 @@ func (s *Service) OrgLinksList(c echo.Context) error {
 		return links.ServerRSSFeed(c.Response(), rss)
 	}
 
-	if c.QueryParam("prev") != "" {
+	if reversed {
 		slices.Reverse(orgLinks)
 	}
 
diff --git a/core/routes_test.go b/core/routes_test.go
index a4c48c7..0c8b320 100644
--- a/core/routes_test.go
+++ b/core/routes_test.go
@@ -3,7 +3,9 @@ package core_test
 import (
 	"bytes"
 	"database/sql"
+	"encoding/json"
 	"fmt"
+	"io"
 	"links"
 	"links/cmd"
 	"links/cmd/test"
@@ -155,6 +157,57 @@ func TestHandlers(t *testing.T) {
 		c.True(strings.Contains(htmlBody, "Link two"))
 	})
 
+	t.Run("recent link list with hostile cursor", func(t *testing.T) {
+		httpmock.Activate()
+		defer httpmock.DeactivateAndReset()
+		jsonResponse, err := httpmock.NewJsonResponder(http.StatusOK, httpmock.File("samples/org_link_list.json"))
+		c.NoError(err)
+		// Stands in for the API, which rejects an undecodable cursor with a bare
+		// "Invalid cursor" error that the web tier used to surface as a 500. The
+		// handler must never forward one, so any after/before variable fails here.
+		httpmock.RegisterResponder("POST", "http://127.0.0.1:8080/query",
+			func(req *http.Request) (*http.Response, error) {
+				body, rerr := io.ReadAll(req.Body)
+				if rerr != nil {
+					return nil, rerr
+				}
+				var payload struct {
+					Variables map[string]any `json:"variables"`
+				}
+				if rerr = json.Unmarshal(body, &payload); rerr != nil {
+					return nil, rerr
+				}
+				for _, key := range []string{"after", "before"} {
+					if _, ok := payload.Variables[key]; ok {
+						return httpmock.NewJsonResponse(http.StatusOK, map[string]any{
+							"data":   nil,
+							"errors": []map[string]any{{"message": "Invalid cursor"}},
+						})
+					}
+				}
+				req.Body = io.NopCloser(bytes.NewReader(body))
+				return jsonResponse(req)
+			})
+
+		hostile := "eyJBZnRlciI6MTk5MDcxLCJCZWZvcmUiOjIwNzc4OSwiTGltaXQiOjI1fQ==\";SELECT SLEEP(20)#"
+		for _, param := range []string{"next", "prev"} {
+			request := httptest.NewRequest(http.MethodGet,
+				"/recent?"+url.Values{param: {hostile}}.Encode(), nil)
+			recorder := httptest.NewRecorder()
+			ctx := &server.Context{
+				Server:  srv,
+				Context: e.NewContext(request, recorder),
+				User:    test.NewTestUser(1, false, false, false, false),
+			}
+			ctx.SetPath("/recent")
+			err = test.MakeRequestWithDomain(srv, coreService.OrgLinksList, ctx, domains[0])
+			c.NoError(err, "hostile %s cursor should not error", param)
+			htmlBody := recorder.Body.String()
+			c.True(strings.Contains(htmlBody, "Link one"))
+			c.True(strings.Contains(htmlBody, "Link two"))
+		}
+	})
+
 	t.Run("create link form error", func(t *testing.T) {
 		httpmock.Activate()
 		defer httpmock.DeactivateAndReset()
diff --git a/helpers.go b/helpers.go
index d191d43..ad2a855 100644
--- a/helpers.go
+++ b/helpers.go
@@ -9,6 +9,7 @@ import (
 	"fmt"
 	"html/template"
 	"io"
+	"links/api/graph/model"
 	"links/domain"
 	"links/internal/localizer"
 	"links/models"
@@ -499,6 +500,25 @@ func ParsePendingBaseURLs(ctx context.Context, userAgent string) error {
 	return nil
 }
 
+func ValidCursor(v string) bool {
+	var cur model.Cursor
+	return cur.UnmarshalGQL(v) == nil
+}
+
+func SetCursorVars(c echo.Context, op *gqlclient.Operation) bool {
+	if v := c.QueryParam("next"); v != "" {
+		if ValidCursor(v) {
+			op.Var("after", v)
+		}
+		return false
+	}
+	if v := c.QueryParam("prev"); v != "" && ValidCursor(v) {
+		op.Var("before", v)
+		return true
+	}
+	return false
+}
+
 // GetPaginationParams returns the params needed for cursor pagination
 func GetPaginationParams(c echo.Context, pagvar, cursor string, exclude ...string) template.URL {
 	q := make(url.Values)
@@ -1340,11 +1360,7 @@ func FetchAuditLogs(c echo.Context, userID int,
 	if limit > 0 {
 		op.Var("limit", limit)
 	}
-	if c.QueryParam("next") != "" {
-		op.Var("after", c.QueryParam("next"))
-	} else if c.QueryParam("prev") != "" {
-		op.Var("before", c.QueryParam("prev"))
-	}
+	reversed := SetCursorVars(c, op)
 
 	var result AuditLogResponse
 	err := Execute(LangContext(c), op, &result)
@@ -1354,7 +1370,7 @@ func FetchAuditLogs(c echo.Context, userID int,
 
 	gctx := c.(*server.Context)
 	user := gctx.User.(*models.User)
-	if c.QueryParam("prev") != "" {
+	if reversed {
 		slices.Reverse(result.AuditLogs.Result)
 	}
 
diff --git a/helpers_test.go b/helpers_test.go
new file mode 100644
index 0000000..0e44115
--- /dev/null
+++ b/helpers_test.go
@@ -0,0 +1,92 @@
+package links
+
+import (
+	"encoding/base64"
+	"links/api/graph/model"
+	"net/http"
+	"net/http/httptest"
+	"net/url"
+	"testing"
+
+	"git.sr.ht/~emersion/gqlclient"
+	"github.com/labstack/echo/v4"
+	"github.com/stretchr/testify/require"
+)
+
+func validCursor(t *testing.T) string {
+	t.Helper()
+	cur := model.Cursor{After: 199071, Before: 207789, Limit: 25}
+	rec := httptest.NewRecorder()
+	cur.MarshalGQL(rec.Body)
+	return rec.Body.String()[1 : rec.Body.Len()-1]
+}
+
+func TestValidCursor(t *testing.T) {
+	c := require.New(t)
+	good := validCursor(t)
+
+	tests := []struct {
+		name  string
+		value string
+		want  bool
+	}{
+		{"round trip", good, true},
+		{"empty", "", false},
+		{"sql injection probe", good + `";SELECT SLEEP(20)#`, false},
+		{"not base64", "not a cursor", false},
+		{"base64 of non json", base64.StdEncoding.EncodeToString([]byte("nope")), false},
+		{"base64 of json array", base64.StdEncoding.EncodeToString([]byte(`[1,2]`)), false},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			c.Equal(tt.want, ValidCursor(tt.value))
+		})
+	}
+}
+
+func TestSetCursorVars(t *testing.T) {
+	good := validCursor(t)
+	bad := good + `";SELECT SLEEP(20)#`
+
+	newCtx := func(query url.Values) echo.Context {
+		req := httptest.NewRequest(http.MethodGet, "/?"+query.Encode(), nil)
+		return echo.New().NewContext(req, httptest.NewRecorder())
+	}
+
+	// gqlclient.Operation keeps its variables unexported and panics when the same
+	// variable is set twice, so re-setting a key is the only way to observe whether
+	// SetCursorVars applied it.
+	assertVarSet := func(t *testing.T, op *gqlclient.Operation, key string, want bool) {
+		t.Helper()
+		set := func() { op.Var(key, "probe") }
+		if want {
+			require.Panics(t, set, "expected %q to have been set", key)
+			return
+		}
+		require.NotPanics(t, set, "expected %q to be unset", key)
+	}
+
+	tests := []struct {
+		name         string
+		query        url.Values
+		wantReversed bool
+		wantAfter    bool
+		wantBefore   bool
+	}{
+		{"no cursor", url.Values{}, false, false, false},
+		{"valid next", url.Values{"next": {good}}, false, true, false},
+		{"valid prev", url.Values{"prev": {good}}, true, false, true},
+		{"invalid next", url.Values{"next": {bad}}, false, false, false},
+		{"invalid prev", url.Values{"prev": {bad}}, false, false, false},
+		{"next wins over prev", url.Values{"next": {good}, "prev": {good}}, false, true, false},
+		{"invalid next does not fall through to prev", url.Values{"next": {bad}, "prev": {good}}, false, false, false},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			op := gqlclient.NewOperation("query { noop }")
+			require.Equal(t, tt.wantReversed, SetCursorVars(newCtx(tt.query), op))
+			assertVarSet(t, op, "after", tt.wantAfter)
+			assertVarSet(t, op, "before", tt.wantBefore)
+		})
+	}
+}
diff --git a/list/routes.go b/list/routes.go
index 25392cc..63328ab 100644
--- a/list/routes.go
+++ b/list/routes.go
@@ -556,11 +556,7 @@ func (s *Service) ListingLinksManage(c echo.Context) error {
 	op.Var("slug", listing.Slug)
 	op.Var("domainId", listing.DomainID)
 
-	if c.QueryParam("next") != "" {
-		op.Var("after", c.QueryParam("next"))
-	} else if c.QueryParam("prev") != "" {
-		op.Var("before", c.QueryParam("prev"))
-	}
+	reversed := links.SetCursorVars(c, op)
 
 	err = links.Execute(c.Request().Context(), op, &result)
 	if err != nil {
@@ -570,7 +566,7 @@ func (s *Service) ListingLinksManage(c echo.Context) error {
 		return err
 	}
 
-	if c.QueryParam("prev") != "" {
+	if reversed {
 		slices.Reverse(result.Listing.Links)
 	}
 
@@ -1099,11 +1095,7 @@ func (s *Service) ListingList(c echo.Context) error {
 		queries.Add("exclude", excludeTag)
 	}
 
-	if c.QueryParam("next") != "" {
-		op.Var("after", c.QueryParam("next"))
-	} else if c.QueryParam("prev") != "" {
-		op.Var("before", c.QueryParam("prev"))
-	}
+	reversed := links.SetCursorVars(c, op)
 
 	var domID int
 	if c.QueryParam("domain") != "" {
@@ -1122,7 +1114,7 @@ func (s *Service) ListingList(c echo.Context) error {
 		}
 		return err
 	}
-	if c.QueryParam("prev") != "" {
+	if reversed {
 		slices.Reverse(result.Listings.Result)
 	}
 
@@ -1640,11 +1632,7 @@ func (r *DetailService) ListDetail(c echo.Context) error {
 	}
 	op.Var("domainId", domain.ID)
 
-	if c.QueryParam("next") != "" {
-		op.Var("after", c.QueryParam("next"))
-	} else if c.QueryParam("prev") != "" {
-		op.Var("before", c.QueryParam("prev"))
-	}
+	links.SetCursorVars(c, op)
 
 	err := links.Execute(c.Request().Context(), op, &result)
 	if err != nil {
diff --git a/short/routes.go b/short/routes.go
index 4c92a39..02e4c26 100644
--- a/short/routes.go
+++ b/short/routes.go
@@ -136,11 +136,7 @@ func (s *Service) LinkShortList(c echo.Context) error {
 		queries.Add("exclude", excludeTag)
 	}
 
-	if c.QueryParam("next") != "" {
-		op.Var("after", c.QueryParam("next"))
-	} else if c.QueryParam("prev") != "" {
-		op.Var("before", c.QueryParam("prev"))
-	}
+	reversed := links.SetCursorVars(c, op)
 
 	var domID int
 	if c.QueryParam("domain") != "" {
@@ -181,7 +177,7 @@ func (s *Service) LinkShortList(c echo.Context) error {
 	pd.Data["domain"] = lt.Translate("Domain")
 
 	linkShorts := result.LinkShorts.Result
-	if c.QueryParam("prev") != "" {
+	if reversed {
 		slices.Reverse(linkShorts)
 	}
 
-- 
2.54.0