chore: replace with container/ring

Signed-off-by: Danny Kopping <danny@coder.com>

coderd/rbac/authz.go

@@ -1,6 +1,7 @@
 package rbac
 
 import (
+	"container/ring"
 	"context"
 	"crypto/sha256"
 	_ "embed"
@@ -801,11 +802,17 @@ func (c *authRecorder) Prepare(ctx context.Context, subject Subject, action poli
 
 type authzCheckRecorderKey struct{}
 
+const (
+	// MaxRecordedChecks is the maximum number of authorization checks to keep
+	// in the ring buffer. This prevents unbounded memory growth for long-running contexts.
+	MaxRecordedChecks = 20
+)
+
 type AuthzCheckRecorder struct {
-	// lock guards checks
+	// lock guards ring buffer
 	lock sync.Mutex
-	// checks is a list preformatted authz check IDs and their result
-	checks []recordedCheck
+	// ring is a ring buffer of preformatted authz check IDs and their result
+	ring *ring.Ring
 }
 
 type recordedCheck struct {
@@ -815,7 +822,9 @@ type recordedCheck struct {
 }
 
 func WithAuthzCheckRecorder(ctx context.Context) context.Context {
-	return context.WithValue(ctx, authzCheckRecorderKey{}, &AuthzCheckRecorder{})
+	return context.WithValue(ctx, authzCheckRecorderKey{}, &AuthzCheckRecorder{
+		ring: ring.New(MaxRecordedChecks),
+	})
 }
 
 func recordAuthzCheck(ctx context.Context, action policy.Action, object Object, authorized bool) {
@@ -824,40 +833,29 @@ func recordAuthzCheck(ctx context.Context, action policy.Action, object Object,
 		return
 	}
 
-	// We serialize the check using the following syntax
-	var b strings.Builder
+	// Build string parts to concatenate.
+	parts := make([]string, 0, 8)
+
 	if object.OrgID != "" {
-		_, err := fmt.Fprintf(&b, "organization:%v::", object.OrgID)
-		if err != nil {
-			return
-		}
+		parts = append(parts, "organization:", object.OrgID, "::")
 	}
 	if object.AnyOrgOwner {
-		_, err := fmt.Fprint(&b, "organization:any::")
-		if err != nil {
-			return
-		}
+		parts = append(parts, "organization:any::")
 	}
 	if object.Owner != "" {
-		_, err := fmt.Fprintf(&b, "owner:%v::", object.Owner)
-		if err != nil {
-			return
-		}
+		parts = append(parts, "owner:", object.Owner, "::")
 	}
 	if object.ID != "" {
-		_, err := fmt.Fprintf(&b, "id:%v::", object.ID)
-		if err != nil {
-			return
-		}
-	}
-	_, err := fmt.Fprintf(&b, "%v.%v", object.RBACObject().Type, action)
-	if err != nil {
-		return
+		parts = append(parts, "id:", object.ID, "::")
 	}
+	parts = append(parts, object.RBACObject().Type, ".", string(action))
+
+	name := strings.Join(parts, "")
 
 	r.lock.Lock()
-	defer r.lock.Unlock()
-	r.checks = append(r.checks, recordedCheck{name: b.String(), result: authorized})
+	r.ring.Value = recordedCheck{name: name, result: authorized}
+	r.ring = r.ring.Next()
+	r.lock.Unlock()
 }
 
 func GetAuthzCheckRecorder(ctx context.Context) (*AuthzCheckRecorder, bool) {
@@ -870,17 +868,30 @@ func GetAuthzCheckRecorder(ctx context.Context) (*AuthzCheckRecorder, bool) {
 }
 
 // String serializes all of the checks recorded, using the following syntax:
+// name1=result1; name2=result2; ...
 func (r *AuthzCheckRecorder) String() string {
 	r.lock.Lock()
 	defer r.lock.Unlock()
 
-	if len(r.checks) == 0 {
+	var checks []string
+	current := r.ring
+	for i := 0; i < MaxRecordedChecks; i++ {
+		if current.Value == nil {
+			current = current.Next()
+			continue
+		}
+		check, ok := current.Value.(recordedCheck)
+		if !ok || check.name == "" {
+			current = current.Next()
+			continue
+		}
+		checks = append(checks, fmt.Sprintf("%v=%v", check.name, check.result))
+		current = current.Next()
+	}
+
+	if len(checks) == 0 {
 		return "nil"
 	}
 
-	checks := make([]string, 0, len(r.checks))
-	for _, check := range r.checks {
-		checks = append(checks, fmt.Sprintf("%v=%v", check.name, check.result))
-	}
 	return strings.Join(checks, "; ")
 }

coderd/rbac/authz_test.go

@@ -314,6 +314,96 @@ func BenchmarkCacher(b *testing.B) {
 	}
 }
 
+// BenchmarkRecorder benchmarks the overhead of recording authorization checks.
+// This is important because recordAuthzCheck is called on every authorization
+// in hot paths and uses string building, fmt operations, and mutex locking.
+//
+//	go test -run=^$ -bench '^BenchmarkRecorder$' -benchmem -cpuprofile profile.out
+func BenchmarkRecorder(b *testing.B) {
+	// Test different object configurations to measure string building cost.
+	objects := []struct {
+		name string
+		obj  rbac.Object
+	}{
+		{
+			name: "Minimal",
+			obj: rbac.Object{
+				Type: "workspace",
+			},
+		},
+		{
+			name: "WithID",
+			obj: rbac.Object{
+				Type: "workspace",
+				ID:   uuid.NewString(),
+			},
+		},
+		{
+			name: "WithOwner",
+			obj: rbac.Object{
+				Type:  "workspace",
+				ID:    uuid.NewString(),
+				Owner: uuid.NewString(),
+			},
+		},
+		{
+			name: "WithOrg",
+			obj: rbac.Object{
+				Type:  "workspace",
+				ID:    uuid.NewString(),
+				Owner: uuid.NewString(),
+				OrgID: uuid.NewString(),
+			},
+		},
+		{
+			name: "AllFields",
+			obj: rbac.Object{
+				Type:  "workspace",
+				ID:    uuid.NewString(),
+				Owner: uuid.NewString(),
+				OrgID: uuid.NewString(),
+			},
+		},
+	}
+
+	for _, obj := range objects {
+		b.Run(obj.name, func(b *testing.B) {
+			ctx := rbac.WithAuthzCheckRecorder(context.Background())
+			authz := rbac.Recorder(&coderdtest.FakeAuthorizer{})
+			subj := coderdtest.RandomRBACSubject()
+			action := policy.ActionRead
+
+			b.ReportAllocs()
+			b.ResetTimer()
+			for b.Loop() {
+				_ = authz.Authorize(ctx, subj, action, obj.obj)
+			}
+		})
+	}
+
+	// Benchmark contention with multiple goroutines.
+	b.Run("Contention", func(b *testing.B) {
+		ctx := rbac.WithAuthzCheckRecorder(context.Background())
+		authz := rbac.Recorder(&coderdtest.FakeAuthorizer{})
+		subj := coderdtest.RandomRBACSubject()
+		action := policy.ActionRead
+		obj := rbac.Object{
+			Type:  "workspace",
+			ID:    uuid.NewString(),
+			Owner: uuid.NewString(),
+			OrgID: uuid.NewString(),
+		}
+
+		b.ReportAllocs()
+		b.ResetTimer()
+		b.RunParallel(func(pb *testing.PB) {
+			for pb.Next() {
+				_ = authz.Authorize(ctx, subj, action, obj)
+			}
+		})
+	})
+}
+
 func TestCache(t *testing.T) {
 	t.Parallel()