tenseleyflow/shithub / 9c72313

Browse files

repos: bump create cap to 20/hour and bypass throttle for site admins

Authored by mfwolffe <wolffemf@dukes.jmu.edu>
SHA
9c72313495be2e194f19942aff318e874c4ea9dc
Parents
c671109
Tree
5cfdf43

2 changed files

StatusFile+-
M internal/repos/create.go 19 10
M internal/repos/create_test.go 60 0
internal/repos/create.gomodified
@@ -28,10 +28,10 @@ import (
2828
 )
2929
3030
 // CreateRateLimit caps how many repos one user can create per hour.
31
-// The spec calls for 10/hour; the value is exported so the operator can
32
-// override via the throttle plumbing if needed.
31
+// 20/hour leaves headroom for bulk-migrating an existing org without
32
+// being a license to spam; site admins bypass the cap entirely.
3333
 const (
34
-	CreateRateLimitMax    = 10
34
+	CreateRateLimitMax    = 20
3535
 	CreateRateLimitWindow = time.Hour
3636
 )
3737
@@ -70,6 +70,12 @@ type Params struct {
7070
 	// OwnerUserID for personal repos when zero.
7171
 	ActorUserID int64
7272
73
+	// ActorIsSiteAdmin, when true, bypasses the per-actor create
74
+	// rate-limit. Site admins are trusted operators (bulk migration,
75
+	// fixture seeding) and the cap exists to deter abuse from regular
76
+	// accounts, not to throttle staff.
77
+	ActorIsSiteAdmin bool
78
+
7379
 	Name        string // already lowercased + trimmed
7480
 	Description string
7581
 	Visibility  string // "public" | "private"
@@ -137,13 +143,16 @@ func Create(ctx context.Context, deps Deps, p Params) (Result, error) {
137143
138144
 	// Rate-limit per actor (NOT per owner) so a user can't bypass the
139145
 	// per-account cap by spreading creates across orgs they manage.
140
-	if err := deps.Limiter.Hit(ctx, deps.Pool, throttle.Limit{
141
-		Scope:      "repo_create",
142
-		Identifier: fmt.Sprintf("user:%d", p.ActorUserID),
143
-		Max:        CreateRateLimitMax,
144
-		Window:     CreateRateLimitWindow,
145
-	}); err != nil {
146
-		return Result{}, err
146
+	// Site admins skip the cap entirely.
147
+	if !p.ActorIsSiteAdmin {
148
+		if err := deps.Limiter.Hit(ctx, deps.Pool, throttle.Limit{
149
+			Scope:      "repo_create",
150
+			Identifier: fmt.Sprintf("user:%d", p.ActorUserID),
151
+			Max:        CreateRateLimitMax,
152
+			Window:     CreateRateLimitWindow,
153
+		}); err != nil {
154
+			return Result{}, err
155
+		}
147156
 	}
148157
149158
 	// Resolve author identity for the initial commit. The actor (the
internal/repos/create_test.gomodified
@@ -5,6 +5,7 @@ package repos_test
55
 import (
66
 	"context"
77
 	"errors"
8
+	"fmt"
89
 	"io"
910
 	"log/slog"
1011
 	"os/exec"
@@ -265,6 +266,65 @@ func TestCreate_OrgOwned(t *testing.T) {
265266
 	}
266267
 }
267268
269
+// TestCreate_ThrottlesNonAdmin saturates the per-actor cap directly via
270
+// the limiter and confirms a non-admin Create call returns the typed
271
+// throttle error. Doing it this way avoids spinning up
272
+// CreateRateLimitMax+1 real repositories.
273
+func TestCreate_ThrottlesNonAdmin(t *testing.T) {
274
+	t.Parallel()
275
+	_, deps, uid, uname, _ := setupCreateEnv(t)
276
+	saturateCreateLimiter(t, deps, uid)
277
+
278
+	_, err := repos.Create(context.Background(), deps, repos.Params{
279
+		OwnerUserID:   uid,
280
+		OwnerUsername: uname,
281
+		Name:          "should-throttle",
282
+		Visibility:    "public",
283
+	})
284
+	if !throttle.IsThrottled(err) {
285
+		t.Fatalf("Create: err = %v, want throttle error", err)
286
+	}
287
+}
288
+
289
+// TestCreate_SiteAdminBypassesThrottle is the bookend: same saturated
290
+// counter, but with ActorIsSiteAdmin=true the create succeeds.
291
+func TestCreate_SiteAdminBypassesThrottle(t *testing.T) {
292
+	t.Parallel()
293
+	_, deps, uid, uname, _ := setupCreateEnv(t)
294
+	saturateCreateLimiter(t, deps, uid)
295
+
296
+	res, err := repos.Create(context.Background(), deps, repos.Params{
297
+		OwnerUserID:      uid,
298
+		OwnerUsername:    uname,
299
+		ActorIsSiteAdmin: true,
300
+		Name:             "admin-bypass",
301
+		Visibility:       "public",
302
+	})
303
+	if err != nil {
304
+		t.Fatalf("Create with admin bypass: %v", err)
305
+	}
306
+	if res.Repo.Name != "admin-bypass" {
307
+		t.Fatalf("created repo name = %q, want admin-bypass", res.Repo.Name)
308
+	}
309
+}
310
+
311
+// saturateCreateLimiter pushes the per-actor counter for "repo_create"
312
+// up to the cap so the next non-admin Hit returns ErrThrottled.
313
+func saturateCreateLimiter(t *testing.T, deps repos.Deps, uid int64) {
314
+	t.Helper()
315
+	lim := throttle.Limit{
316
+		Scope:      "repo_create",
317
+		Identifier: fmt.Sprintf("user:%d", uid),
318
+		Max:        repos.CreateRateLimitMax,
319
+		Window:     repos.CreateRateLimitWindow,
320
+	}
321
+	for i := 0; i < repos.CreateRateLimitMax; i++ {
322
+		if err := deps.Limiter.Hit(context.Background(), deps.Pool, lim); err != nil {
323
+			t.Fatalf("priming limiter hit %d: %v", i, err)
324
+		}
325
+	}
326
+}
327
+
268328
 func TestCreate_RejectsBothOwnerKindsSet(t *testing.T) {
269329
 	t.Parallel()
270330
 	_, deps, uid, uname, _ := setupCreateEnv(t)