tenseleyflow/shithub / 1fe08a6

Browse files

web/handlers/repo: branch protection per-feature enforce flip for users

Authored by mfwolffe <wolffemf@dukes.jmu.edu>
SHA
1fe08a6b80dea9f9342c94e816a4dff08a870b64
Parents
0f243b6
Tree
f891a27

4 changed files

StatusFile+-
M internal/web/handlers/repo/repo.go 7 0
M internal/web/handlers/repo/repo_test.go 17 7
M internal/web/handlers/repo/settings_branches.go 25 4
A internal/web/handlers/repo/settings_branches_pro07_test.go 192 0
internal/web/handlers/repo/repo.gomodified
@@ -25,6 +25,7 @@ import (
2525
 	"github.com/tenseleyFlow/shithub/internal/auth/throttle"
2626
 	checksdb "github.com/tenseleyFlow/shithub/internal/checks/sqlc"
2727
 	"github.com/tenseleyFlow/shithub/internal/entitlements"
28
+	"github.com/tenseleyFlow/shithub/internal/infra/config"
2829
 	"github.com/tenseleyFlow/shithub/internal/infra/storage"
2930
 	issuesdb "github.com/tenseleyFlow/shithub/internal/issues/sqlc"
3031
 	"github.com/tenseleyFlow/shithub/internal/orgs"
@@ -82,6 +83,12 @@ type Deps struct {
8283
 	// have hook shims pointing at the right binary. Empty in test fixtures
8384
 	// that don't exercise hooks.
8485
 	ShithubdPath string
86
+	// BillingEnforce carries PRO07's per-feature enforcement flags.
87
+	// Branch-protection gating on personal private repos consults
88
+	// these to decide whether a user-kind would-deny should block the
89
+	// save (enforce=true) or log a report-only event and proceed
90
+	// (PRO05 default, enforce=false).
91
+	BillingEnforce config.EnforceConfig
8592
 }
8693
 
8794
 // Handlers is the registered handler set. Construct via New.
internal/web/handlers/repo/repo_test.gomodified
@@ -32,6 +32,7 @@ import (
3232
 	"github.com/tenseleyFlow/shithub/internal/auth/audit"
3333
 	"github.com/tenseleyFlow/shithub/internal/auth/policy"
3434
 	"github.com/tenseleyFlow/shithub/internal/auth/throttle"
35
+	"github.com/tenseleyFlow/shithub/internal/infra/config"
3536
 	"github.com/tenseleyFlow/shithub/internal/infra/storage"
3637
 	reposdb "github.com/tenseleyFlow/shithub/internal/repos/sqlc"
3738
 	"github.com/tenseleyFlow/shithub/internal/testing/dbtest"
@@ -60,6 +61,14 @@ type repoFixture struct {
6061
 // newRepoFixture sets up a Handlers wired to a fresh test DB with two
6162
 // users (owner alice, stranger bob) and two repos (public + private).
6263
 func newRepoFixture(t *testing.T) *repoFixture {
64
+	return newRepoFixtureWithEnforce(t, config.EnforceConfig{})
65
+}
66
+
67
+// newRepoFixtureWithEnforce mirrors newRepoFixture but plumbs an
68
+// operator-configured per-feature enforce matrix into the handler
69
+// Deps. PRO07 tests pass a config with the relevant user-kind flag
70
+// flipped to true to exercise the enforce path.
71
+func newRepoFixtureWithEnforce(t *testing.T, enforce config.EnforceConfig) *repoFixture {
6372
 	t.Helper()
6473
 	pool := dbtest.NewTestDB(t)
6574
 
@@ -74,13 +83,14 @@ func newRepoFixture(t *testing.T) *repoFixture {
7483
 	objectStore := storage.NewMemoryStore()
7584
 
7685
 	h, err := New(Deps{
77
-		Logger:      slog.New(slog.NewTextHandler(io.Discard, nil)),
78
-		Render:      rr,
79
-		Pool:        pool,
80
-		RepoFS:      rfs,
81
-		ObjectStore: objectStore,
82
-		Audit:       audit.NewRecorder(),
83
-		Limiter:     throttle.NewLimiter(),
86
+		Logger:         slog.New(slog.NewTextHandler(io.Discard, nil)),
87
+		Render:         rr,
88
+		Pool:           pool,
89
+		RepoFS:         rfs,
90
+		ObjectStore:    objectStore,
91
+		Audit:          audit.NewRecorder(),
92
+		Limiter:        throttle.NewLimiter(),
93
+		BillingEnforce: enforce,
8494
 	})
8595
 	if err != nil {
8696
 		t.Fatalf("New: %v", err)
internal/web/handlers/repo/settings_branches.gomodified
@@ -245,9 +245,15 @@ func principalFromRepo(row reposdb.Repo) (billing.Principal, bool) {
245245
 // report-only-allowed); a non-empty code means the save is blocked
246246
 // and the caller redirects to the settings page with a banner.
247247
 //
248
-// Report-only semantics: user-kind would-denies log via Logger and
249
-// return empty notice. Org-kind would-denies return the SP05
250
-// notice code.
248
+// Enforcement semantics:
249
+//   - Org kind: SP05 enforcement — would-denies return the notice
250
+//     code unconditionally.
251
+//   - User kind, PRO07 enforce flag off (default): logs the would-
252
+//     deny via Logger and returns empty notice (report-only).
253
+//   - User kind, PRO07 enforce flag on: returns the notice code
254
+//     (blocks the save). The flag is per-feature; see
255
+//     EnforceConfig.UserAdvancedBranchProtection and
256
+//     EnforceConfig.UserRequiredReviewers in internal/infra/config.
251257
 func (h *Handlers) evaluateBranchProtectionFeature(ctx context.Context, p billing.Principal, feature entitlements.Feature, requiredReviewers bool) (string, error) {
252258
 	if !entitlements.FeatureAppliesToKind(feature, p.Kind) {
253259
 		return "", nil
@@ -259,7 +265,7 @@ func (h *Handlers) evaluateBranchProtectionFeature(ctx context.Context, p billin
259265
 	if decision.Allowed {
260266
 		return "", nil
261267
 	}
262
-	if p.IsUser() {
268
+	if p.IsUser() && !h.userBranchProtectionEnforceOn(feature) {
263269
 		h.d.Logger.InfoContext(ctx, "entitlements.report_only_deny",
264270
 			"principal", p.String(),
265271
 			"principal_kind", string(p.Kind),
@@ -272,6 +278,21 @@ func (h *Handlers) evaluateBranchProtectionFeature(ctx context.Context, p billin
272278
 	return branchProtectionNoticeCode(decision, requiredReviewers), nil
273279
 }
274280
 
281
+// userBranchProtectionEnforceOn maps a feature to the operator's
282
+// per-feature enforce knob. Unknown features default to false (safe:
283
+// stay in report-only). PRO07 ships with both flags defaulting to
284
+// false; operators flip per feature after the 7-day telemetry soak.
285
+func (h *Handlers) userBranchProtectionEnforceOn(feature entitlements.Feature) bool {
286
+	switch feature {
287
+	case entitlements.FeatureRequiredReviewers:
288
+		return h.d.BillingEnforce.UserRequiredReviewers
289
+	case entitlements.FeatureAdvancedBranchProtection:
290
+		return h.d.BillingEnforce.UserAdvancedBranchProtection
291
+	default:
292
+		return false
293
+	}
294
+}
295
+
275296
 func branchProtectionNoticeCode(decision entitlements.Decision, requiredReviewers bool) string {
276297
 	if requiredReviewers {
277298
 		switch decision.Reason {
internal/web/handlers/repo/settings_branches_pro07_test.goadded
@@ -0,0 +1,192 @@
1
+// SPDX-License-Identifier: AGPL-3.0-or-later
2
+
3
+package repo
4
+
5
+import (
6
+	"context"
7
+	"net/http/httptest"
8
+	"net/url"
9
+	"strconv"
10
+	"testing"
11
+	"time"
12
+
13
+	"github.com/jackc/pgx/v5/pgtype"
14
+
15
+	billingdb "github.com/tenseleyFlow/shithub/internal/billing/sqlc"
16
+	"github.com/tenseleyFlow/shithub/internal/infra/config"
17
+	reposdb "github.com/tenseleyFlow/shithub/internal/repos/sqlc"
18
+)
19
+
20
+// PRO07 — user-kind branch protection enforce flip.
21
+//
22
+// PRO05 plumbed user-kind report-only logging through
23
+// evaluateBranchProtectionFeature. PRO07 wires
24
+// EnforceConfig.User{RequiredReviewers,AdvancedBranchProtection} into
25
+// the same site so operators can flip per-feature enforcement after
26
+// the telemetry soak. These tests pin both modes.
27
+
28
+func TestSettingsBranches_UserKindReportOnlyRequiredReviewersAllowsSave(t *testing.T) {
29
+	t.Parallel()
30
+	f := newRepoFixture(t) // zero-value enforce ⇒ report-only
31
+	mux := f.branchesSettingsMux(f.owner.ID, f.owner.Username)
32
+
33
+	resp := httptest.NewRecorder()
34
+	req := newFormRequest("POST", "/alice/private-repo/settings/branches", url.Values{
35
+		"pattern":               {"trunk"},
36
+		"required_review_count": {"2"},
37
+	})
38
+	mux.ServeHTTP(resp, req)
39
+	if resp.Code != 303 {
40
+		t.Fatalf("status=%d body=%s", resp.Code, resp.Body.String())
41
+	}
42
+	// Report-only ⇒ no notice code; redirect lands on the saved
43
+	// confirmation page.
44
+	if got := resp.Header().Get("Location"); got != "/alice/private-repo/settings/branches?notice=saved" {
45
+		t.Fatalf("expected report-only save, got redirect=%q", got)
46
+	}
47
+	assertBranchProtectionRuleCount(t, f, f.privateRepo.ID, 1)
48
+}
49
+
50
+func TestSettingsBranches_UserKindEnforceFlipRequiredReviewersBlocks(t *testing.T) {
51
+	t.Parallel()
52
+	f := newRepoFixtureWithEnforce(t, config.EnforceConfig{
53
+		UserRequiredReviewers: true,
54
+	})
55
+	mux := f.branchesSettingsMux(f.owner.ID, f.owner.Username)
56
+
57
+	resp := httptest.NewRecorder()
58
+	req := newFormRequest("POST", "/alice/private-repo/settings/branches", url.Values{
59
+		"pattern":               {"trunk"},
60
+		"required_review_count": {"2"},
61
+	})
62
+	mux.ServeHTTP(resp, req)
63
+	if resp.Code != 303 {
64
+		t.Fatalf("status=%d body=%s", resp.Code, resp.Body.String())
65
+	}
66
+	if got := resp.Header().Get("Location"); got != "/alice/private-repo/settings/branches?notice=required-reviewers-upgrade" {
67
+		t.Fatalf("expected upgrade notice, got %q", got)
68
+	}
69
+	assertBranchProtectionRuleCount(t, f, f.privateRepo.ID, 0)
70
+}
71
+
72
+func TestSettingsBranches_UserKindEnforceFlipAdvancedChecksBlocks(t *testing.T) {
73
+	t.Parallel()
74
+	f := newRepoFixtureWithEnforce(t, config.EnforceConfig{
75
+		UserAdvancedBranchProtection: true,
76
+	})
77
+	mux := f.branchesSettingsMux(f.owner.ID, f.owner.Username)
78
+
79
+	resp := httptest.NewRecorder()
80
+	req := newFormRequest("POST", "/alice/private-repo/settings/branches", url.Values{
81
+		"pattern":                     {"trunk"},
82
+		"required_status_check_names": {"ci"},
83
+	})
84
+	mux.ServeHTTP(resp, req)
85
+	if resp.Code != 303 {
86
+		t.Fatalf("status=%d body=%s", resp.Code, resp.Body.String())
87
+	}
88
+	if got := resp.Header().Get("Location"); got != "/alice/private-repo/settings/branches?notice=branch-protection-upgrade" {
89
+		t.Fatalf("expected upgrade notice, got %q", got)
90
+	}
91
+	assertBranchProtectionRuleCount(t, f, f.privateRepo.ID, 0)
92
+}
93
+
94
+// TestSettingsBranches_UserKindEnforceFlipDoesNotAffectPublicRepos
95
+// locks the visibility short-circuit: enforce flags fire only on
96
+// private personal repos. Public personal repos stay ungated.
97
+func TestSettingsBranches_UserKindEnforceFlipDoesNotAffectPublicRepos(t *testing.T) {
98
+	t.Parallel()
99
+	f := newRepoFixtureWithEnforce(t, config.EnforceConfig{
100
+		UserRequiredReviewers:        true,
101
+		UserAdvancedBranchProtection: true,
102
+	})
103
+	mux := f.branchesSettingsMux(f.owner.ID, f.owner.Username)
104
+
105
+	resp := httptest.NewRecorder()
106
+	req := newFormRequest("POST", "/alice/public-repo/settings/branches", url.Values{
107
+		"pattern":               {"trunk"},
108
+		"required_review_count": {"2"},
109
+	})
110
+	mux.ServeHTTP(resp, req)
111
+	if resp.Code != 303 {
112
+		t.Fatalf("status=%d body=%s", resp.Code, resp.Body.String())
113
+	}
114
+	if got := resp.Header().Get("Location"); got != "/alice/public-repo/settings/branches?notice=saved" {
115
+		t.Fatalf("public repo should bypass gate, got %q", got)
116
+	}
117
+}
118
+
119
+// TestSettingsBranches_UserKindEnforceFlipAllowsProUser verifies the
120
+// gate is keyed on Plan, not just kind: a Pro personal account
121
+// completes the save even with the enforce flag flipped.
122
+func TestSettingsBranches_UserKindEnforceFlipAllowsProUser(t *testing.T) {
123
+	t.Parallel()
124
+	f := newRepoFixtureWithEnforce(t, config.EnforceConfig{
125
+		UserRequiredReviewers: true,
126
+	})
127
+	upgradeRepoFixtureOwnerToPro(t, f)
128
+	mux := f.branchesSettingsMux(f.owner.ID, f.owner.Username)
129
+
130
+	resp := httptest.NewRecorder()
131
+	req := newFormRequest("POST", "/alice/private-repo/settings/branches", url.Values{
132
+		"pattern":               {"trunk"},
133
+		"required_review_count": {"2"},
134
+	})
135
+	mux.ServeHTTP(resp, req)
136
+	if resp.Code != 303 {
137
+		t.Fatalf("status=%d body=%s", resp.Code, resp.Body.String())
138
+	}
139
+	if got := resp.Header().Get("Location"); got != "/alice/private-repo/settings/branches?notice=saved" {
140
+		t.Fatalf("Pro user with enforce should save, got %q", got)
141
+	}
142
+	assertBranchProtectionRuleCount(t, f, f.privateRepo.ID, 1)
143
+}
144
+
145
+// TestSettingsBranches_OrgKindUnchangedByPRO07Flags is the regression
146
+// guard: PRO07's per-feature flags target user kind only. Org-kind
147
+// gating has been on since SP05 and must not change shape when the
148
+// user-kind flags flip.
149
+func TestSettingsBranches_OrgKindUnchangedByPRO07Flags(t *testing.T) {
150
+	t.Parallel()
151
+	for _, enforce := range []config.EnforceConfig{
152
+		{},
153
+		{UserRequiredReviewers: true, UserAdvancedBranchProtection: true},
154
+	} {
155
+		f := newRepoFixtureWithEnforce(t, enforce)
156
+		orgID := f.insertOwnedOrg(t, "acme")
157
+		repo := f.insertOrgRepo(t, orgID, "private-org-repo", reposdb.RepoVisibilityPrivate)
158
+		mux := f.branchesSettingsMux(f.owner.ID, f.owner.Username)
159
+
160
+		resp := httptest.NewRecorder()
161
+		req := newFormRequest("POST", "/acme/private-org-repo/settings/branches", url.Values{
162
+			"pattern":               {"trunk"},
163
+			"required_review_count": {"2"},
164
+		})
165
+		mux.ServeHTTP(resp, req)
166
+		if resp.Code != 303 {
167
+			t.Fatalf("enforce=%+v status=%d body=%s", enforce, resp.Code, resp.Body.String())
168
+		}
169
+		if got := resp.Header().Get("Location"); got != "/acme/private-org-repo/settings/branches?notice=required-reviewers-upgrade" {
170
+			t.Errorf("enforce=%+v: org redirect=%q, want upgrade notice", enforce, got)
171
+		}
172
+		assertBranchProtectionRuleCount(t, f, repo.ID, 0)
173
+	}
174
+}
175
+
176
+func upgradeRepoFixtureOwnerToPro(t *testing.T, f *repoFixture) {
177
+	t.Helper()
178
+	now := time.Now().UTC()
179
+	suffix := strconv.FormatInt(f.owner.ID, 10)
180
+	_, err := billingdb.New().ApplyUserSubscriptionSnapshot(context.Background(), f.pool, billingdb.ApplyUserSubscriptionSnapshotParams{
181
+		UserID:               f.owner.ID,
182
+		Plan:                 billingdb.UserPlanPro,
183
+		SubscriptionStatus:   billingdb.BillingSubscriptionStatusActive,
184
+		StripeSubscriptionID: pgtype.Text{String: "sub_bp_pro_" + suffix, Valid: true},
185
+		CurrentPeriodStart:   pgtype.Timestamptz{Time: now.Add(-time.Hour), Valid: true},
186
+		CurrentPeriodEnd:     pgtype.Timestamptz{Time: now.Add(30 * 24 * time.Hour), Valid: true},
187
+		LastWebhookEventID:   "evt_bp_pro_" + suffix,
188
+	})
189
+	if err != nil {
190
+		t.Fatalf("upgrade owner to Pro: %v", err)
191
+	}
192
+}