From 1e4a76643d026616c28930b5fa31cebd3c0762fc Mon Sep 17 00:00:00 2001 From: Hein Date: Thu, 1 Oct 2026 09:31:56 +0200 Subject: [PATCH] fix(security): apply row security to update and delete queries ApplyRowSecurity only accepted common.SelectQuery, so the BeforeScan hook failed closed on update/delete when a row-security template existed. Type-switch on SelectQuery, UpdateQuery and DeleteQuery; other types still return an error. --- pkg/security/hooks.go | 11 +++++--- pkg/security/hooks_test.go | 53 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+), 3 deletions(-) diff --git a/pkg/security/hooks.go b/pkg/security/hooks.go index 697d769..8b6b300 100644 --- a/pkg/security/hooks.go +++ b/pkg/security/hooks.go @@ -140,11 +140,16 @@ func applyRowSecurity(secCtx SecurityContext, securityList *SecurityList) error // A filter that cannot be attached must fail the request; silently // skipping it would expose every row. - selectQuery, ok := secCtx.GetQuery().(common.SelectQuery) - if !ok { + switch q := secCtx.GetQuery().(type) { + case common.SelectQuery: + secCtx.SetQuery(q.Where(whereClause, whereArgs...)) + case common.UpdateQuery: + secCtx.SetQuery(q.Where(whereClause, whereArgs...)) + case common.DeleteQuery: + secCtx.SetQuery(q.Where(whereClause, whereArgs...)) + default: return fmt.Errorf("row security: query type %T on %s.%s does not support Where", secCtx.GetQuery(), schema, tablename) } - secCtx.SetQuery(selectQuery.Where(whereClause, whereArgs...)) } return nil diff --git a/pkg/security/hooks_test.go b/pkg/security/hooks_test.go index f903ee3..2a0c59f 100644 --- a/pkg/security/hooks_test.go +++ b/pkg/security/hooks_test.go @@ -214,6 +214,32 @@ func (q *recordingQuery) Where(query string, args ...interface{}) common.SelectQ return q } +// recordingUpdateQuery is a common.UpdateQuery that records Where calls. +type recordingUpdateQuery struct { + common.UpdateQuery + clauses []string + args [][]any +} + +func (q *recordingUpdateQuery) Where(query string, args ...interface{}) common.UpdateQuery { + q.clauses = append(q.clauses, query) + q.args = append(q.args, args) + return q +} + +// recordingDeleteQuery is a common.DeleteQuery that records Where calls. +type recordingDeleteQuery struct { + common.DeleteQuery + clauses []string + args [][]any +} + +func (q *recordingDeleteQuery) Where(query string, args ...interface{}) common.DeleteQuery { + q.clauses = append(q.clauses, query) + q.args = append(q.args, args) + return q +} + // Test applyRowSecurity func TestApplyRowSecurity(t *testing.T) { type TestModel struct { @@ -278,6 +304,33 @@ func TestApplyRowSecurity(t *testing.T) { } }) + t.Run("filter is attached to update and delete queries", func(t *testing.T) { + provider := &mockSecurityProvider{rowSecurity: RowSecurity{ + Schema: "public", Tablename: "orders", Template: "user_id = {UserID}", UserID: 1, + }} + secList, _ := NewSecurityList(provider) + ctx := context.Background() + _, _ = secList.LoadRowSecurity(ctx, 1, "public", "orders", false) + + uq := &recordingUpdateQuery{} + dq := &recordingDeleteQuery{} + for name, q := range map[string]interface{}{"update": uq, "delete": dq} { + secCtx := &mockSecurityContext{ + ctx: ctx, userID: 1, hasUser: true, schema: "public", entity: "orders", + model: &TestModel{}, query: q, + } + if err := ApplyRowSecurity(secCtx, secList); err != nil { + t.Fatalf("%s: expected no error, got %v", name, err) + } + } + if len(uq.clauses) != 1 || uq.clauses[0] != "user_id = ?" || uq.args[0][0] != 1 { + t.Fatalf("update: filter not attached correctly: %v %v", uq.clauses, uq.args) + } + if len(dq.clauses) != 1 || dq.clauses[0] != "user_id = ?" || dq.args[0][0] != 1 { + t.Fatalf("delete: filter not attached correctly: %v %v", dq.clauses, dq.args) + } + }) + t.Run("user context is bound as its id, never rendered into SQL", func(t *testing.T) { uc := &UserContext{UserID: 7, SessionID: "sess_secret", UserName: "x' OR '1'='1"} provider := &mockSecurityProvider{rowSecurity: RowSecurity{