tenseleyflow/shithub / 810896d

Browse files

Gate issue and PR controls by policy

Authored by espadonne
SHA
810896d6852e77c122fd8d124477b4742b835b56
Parents
b9c83c2
Tree
731fcd4

9 changed files

StatusFile+-
M docs/internal/issues.md 9 0
M docs/internal/pr-review.md 6 0
M internal/web/handlers/repo/issues.go 28 12
M internal/web/handlers/repo/labels_milestones.go 18 10
M internal/web/handlers/repo/pulls.go 12 0
M internal/web/templates/repo/issue_view.html 20 5
M internal/web/templates/repo/labels.html 2 2
M internal/web/templates/repo/milestones.html 2 2
M internal/web/templates/repo/pull_view.html 9 5
docs/internal/issues.mdmodified
@@ -90,6 +90,15 @@ Issue creation and commenting follow GitHub's public-participation
9090
 model: any logged-in user may open or comment on issues in a public
9191
 repo, while private repos require `read` access.
9292
93
+The issue page is capability-driven:
94
+
95
+- The comment box appears only when `policy.Can(issue:comment)` allows
96
+  and the issue is not locked, unless the viewer is triage+.
97
+- Close/reopen appears for triage+ users and for the issue author.
98
+- Labels, assignees, milestones, and lock controls appear only for the
99
+  matching triage-level policy action. Public participants should not
100
+  see forms that only lead to 403s.
101
+
93102
 ## Cross-reference indexing
94103
95104
 `internal/issues/references.go::insertReferencesFromBody` parses
docs/internal/pr-review.mdmodified
@@ -85,6 +85,12 @@ without starting a review" path) is `pending=false` from the start,
8585
 The settings/branches handler now also accepts
8686
 `required_review_count` and `dismiss_stale_reviews_on_push`.
8787
88
+The PR template hides review-request, review-submit, inline-comment,
89
+and thread-resolve controls unless `policy.Can(pull:review)` allows
90
+the viewer. Merge and close controls are similarly driven by
91
+`pull:merge` and `pull:close` decisions. Public viewers who can read a
92
+PR should not see forms that only lead to 403s.
93
+
8894
 ## Required-review gate
8995
9096
 `internal/pulls/review/required.go::Evaluate` is the authoritative
internal/web/handlers/repo/issues.gomodified
@@ -288,21 +288,37 @@ func (h *Handlers) issueView(w http.ResponseWriter, r *http.Request) {
288288
 			authorName = u.Username
289289
 		}
290290
 	}
291
+	viewer := middleware.CurrentUserFromContext(r.Context())
292
+	actor := policy.UserActor(viewer.ID, viewer.Username, viewer.IsSuspended, false)
293
+	pdeps := policy.Deps{Pool: h.d.Pool}
294
+	repoRef := policy.NewRepoRefFromRepo(row)
295
+	stateRef := repoRef
296
+	if issue.AuthorUserID.Valid {
297
+		stateRef.AuthorUserID = issue.AuthorUserID.Int64
298
+	}
299
+	canCommentAction := policy.Can(r.Context(), pdeps, actor, policy.ActionIssueComment, repoRef).Allow
300
+	canCommentThroughLock := policy.HasRoleAtLeast(r.Context(), pdeps, actor, repoRef, policy.RoleTriage)
291301
292302
 	w.Header().Set("Content-Type", "text/html; charset=utf-8")
293303
 	_ = h.d.Render.RenderPage(w, r, "repo/issue_view", map[string]any{
294
-		"Title":      issue.Title + " · " + row.Name,
295
-		"Owner":      owner.Username,
296
-		"Repo":       row,
297
-		"Issue":      issue,
298
-		"AuthorName": authorName,
299
-		"Comments":   cs,
300
-		"Events":     events,
301
-		"Labels":     labels,
302
-		"Assignees":  assignees,
303
-		"AllLabels":  allLabels,
304
-		"Milestones": milestones,
305
-		"CSRFToken":  middleware.CSRFTokenForRequest(r),
304
+		"Title":                 issue.Title + " · " + row.Name,
305
+		"Owner":                 owner.Username,
306
+		"Repo":                  row,
307
+		"Issue":                 issue,
308
+		"AuthorName":            authorName,
309
+		"Comments":              cs,
310
+		"Events":                events,
311
+		"Labels":                labels,
312
+		"Assignees":             assignees,
313
+		"AllLabels":             allLabels,
314
+		"Milestones":            milestones,
315
+		"CanComment":            canCommentAction && (!issue.Locked || canCommentThroughLock),
316
+		"CanSetIssueState":      policy.Can(r.Context(), pdeps, actor, policy.ActionIssueClose, stateRef).Allow,
317
+		"CanEditIssueLabels":    policy.Can(r.Context(), pdeps, actor, policy.ActionIssueLabel, repoRef).Allow,
318
+		"CanEditIssueAssignees": policy.Can(r.Context(), pdeps, actor, policy.ActionIssueAssign, repoRef).Allow,
319
+		"CanEditIssueMilestone": policy.Can(r.Context(), pdeps, actor, policy.ActionIssueLabel, repoRef).Allow,
320
+		"CanLockIssue":          policy.Can(r.Context(), pdeps, actor, policy.ActionIssueClose, repoRef).Allow,
321
+		"CSRFToken":             middleware.CSRFTokenForRequest(r),
306322
 	})
307323
 }
308324
internal/web/handlers/repo/labels_milestones.gomodified
@@ -24,13 +24,17 @@ func (h *Handlers) labelsList(w http.ResponseWriter, r *http.Request) {
2424
 		return
2525
 	}
2626
 	labels, _ := h.iq.ListLabels(r.Context(), h.d.Pool, row.ID)
27
+	viewer := middleware.CurrentUserFromContext(r.Context())
28
+	actor := policy.UserActor(viewer.ID, viewer.Username, viewer.IsSuspended, false)
29
+	canManage := policy.Can(r.Context(), policy.Deps{Pool: h.d.Pool}, actor, policy.ActionIssueLabel, policy.NewRepoRefFromRepo(row)).Allow
2730
 	w.Header().Set("Content-Type", "text/html; charset=utf-8")
2831
 	_ = h.d.Render.RenderPage(w, r, "repo/labels", map[string]any{
29
-		"Title":     "Labels · " + row.Name,
30
-		"Owner":     owner.Username,
31
-		"Repo":      row,
32
-		"Labels":    labels,
33
-		"CSRFToken": middleware.CSRFTokenForRequest(r),
32
+		"Title":          "Labels · " + row.Name,
33
+		"Owner":          owner.Username,
34
+		"Repo":           row,
35
+		"Labels":         labels,
36
+		"CanManageIssue": canManage,
37
+		"CSRFToken":      middleware.CSRFTokenForRequest(r),
3438
 	})
3539
 }
3640
@@ -119,13 +123,17 @@ func (h *Handlers) milestonesList(w http.ResponseWriter, r *http.Request) {
119123
 		return
120124
 	}
121125
 	ms, _ := h.iq.ListMilestones(r.Context(), h.d.Pool, row.ID)
126
+	viewer := middleware.CurrentUserFromContext(r.Context())
127
+	actor := policy.UserActor(viewer.ID, viewer.Username, viewer.IsSuspended, false)
128
+	canManage := policy.Can(r.Context(), policy.Deps{Pool: h.d.Pool}, actor, policy.ActionIssueLabel, policy.NewRepoRefFromRepo(row)).Allow
122129
 	w.Header().Set("Content-Type", "text/html; charset=utf-8")
123130
 	_ = h.d.Render.RenderPage(w, r, "repo/milestones", map[string]any{
124
-		"Title":      "Milestones · " + row.Name,
125
-		"Owner":      owner.Username,
126
-		"Repo":       row,
127
-		"Milestones": ms,
128
-		"CSRFToken":  middleware.CSRFTokenForRequest(r),
131
+		"Title":          "Milestones · " + row.Name,
132
+		"Owner":          owner.Username,
133
+		"Repo":           row,
134
+		"Milestones":     ms,
135
+		"CanManageIssue": canManage,
136
+		"CSRFToken":      middleware.CSRFTokenForRequest(r),
129137
 	})
130138
 }
131139
internal/web/handlers/repo/pulls.gomodified
@@ -281,6 +281,18 @@ func (h *Handlers) renderPullPage(w http.ResponseWriter, r *http.Request, tab st
281281
 		"Tab":        tab,
282282
 		"CSRFToken":  middleware.CSRFTokenForRequest(r),
283283
 	}
284
+	viewer := middleware.CurrentUserFromContext(r.Context())
285
+	actor := policy.UserActor(viewer.ID, viewer.Username, viewer.IsSuspended, false)
286
+	pdeps := policy.Deps{Pool: h.d.Pool}
287
+	repoRef := policy.NewRepoRefFromRepo(row)
288
+	stateRef := repoRef
289
+	if pr.IAuthorUserID.Valid {
290
+		stateRef.AuthorUserID = pr.IAuthorUserID.Int64
291
+	}
292
+	data["CanReviewPull"] = policy.Can(r.Context(), pdeps, actor, policy.ActionPullReview, repoRef).Allow
293
+	data["CanMergePull"] = policy.Can(r.Context(), pdeps, actor, policy.ActionPullMerge, repoRef).Allow
294
+	data["CanSetPullState"] = policy.Can(r.Context(), pdeps, actor, policy.ActionPullClose, stateRef).Allow
295
+	data["CanReadyPull"] = policy.Can(r.Context(), pdeps, actor, policy.ActionPullCreate, repoRef).Allow
284296
 	for k, v := range extras {
285297
 		data[k] = v
286298
 	}
internal/web/templates/repo/issue_view.htmlmodified
@@ -49,7 +49,7 @@
4949
       </div>
5050
       {{ end }}
5151
52
-      {{ if .Viewer.ID }}
52
+      {{ if .CanComment }}
5353
       <form method="post" action="/{{ .Owner }}/{{ .Repo.Name }}/issues/{{ .Issue.Number }}/comments" class="shithub-comment-form">
5454
         <input type="hidden" name="csrf_token" value="{{ .CSRFToken }}">
5555
         <label>
@@ -57,14 +57,29 @@
5757
           <textarea name="body" rows="6" maxlength="65535" required></textarea>
5858
         </label>
5959
         <div class="shithub-form-actions">
60
+          {{ if .CanSetIssueState }}
6061
           {{ if eq (printf "%s" .Issue.State) "open" }}
6162
           <button type="submit" formaction="/{{ .Owner }}/{{ .Repo.Name }}/issues/{{ .Issue.Number }}/state" name="state" value="closed" class="shithub-button">Close issue</button>
6263
           {{ else }}
6364
           <button type="submit" formaction="/{{ .Owner }}/{{ .Repo.Name }}/issues/{{ .Issue.Number }}/state" name="state" value="open" class="shithub-button">Reopen issue</button>
6465
           {{ end }}
66
+          {{ end }}
6567
           <button type="submit" class="shithub-button shithub-button-primary">Comment</button>
6668
         </div>
6769
       </form>
70
+      {{ else if .CanSetIssueState }}
71
+      <form method="post" action="/{{ .Owner }}/{{ .Repo.Name }}/issues/{{ .Issue.Number }}/state" class="shithub-comment-form">
72
+        <input type="hidden" name="csrf_token" value="{{ .CSRFToken }}">
73
+        <div class="shithub-form-actions">
74
+          {{ if eq (printf "%s" .Issue.State) "open" }}
75
+          <button type="submit" name="state" value="closed" class="shithub-button">Close issue</button>
76
+          {{ else }}
77
+          <button type="submit" name="state" value="open" class="shithub-button">Reopen issue</button>
78
+          {{ end }}
79
+        </div>
80
+      </form>
81
+      {{ else if .Viewer.ID }}
82
+        {{ if .Issue.Locked }}<p class="shithub-issue-signedout">This conversation is locked.</p>{{ end }}
6883
       {{ else }}
6984
       <p class="shithub-issue-signedout"><a href="/login">Sign in</a> to comment.</p>
7085
       {{ end }}
@@ -76,7 +91,7 @@
7691
         {{ if .Labels }}
7792
           {{ range .Labels }}<span class="shithub-label" style="background-color: #{{ .Color }}">{{ .Name }}</span>{{ end }}
7893
         {{ else }}<p class="shithub-muted">None yet</p>{{ end }}
79
-        {{ if .Viewer.ID }}
94
+        {{ if .CanEditIssueLabels }}
8095
         <details>
8196
           <summary>Edit labels</summary>
8297
           <form method="post" action="/{{ .Owner }}/{{ .Repo.Name }}/issues/{{ .Issue.Number }}/labels">
@@ -101,7 +116,7 @@
101116
         {{ if .Assignees }}
102117
           {{ range .Assignees }}<a href="/{{ .Username }}">@{{ .Username }}</a>{{ end }}
103118
         {{ else }}<p class="shithub-muted">No one assigned</p>{{ end }}
104
-        {{ if .Viewer.ID }}
119
+        {{ if .CanEditIssueAssignees }}
105120
         <form method="post" action="/{{ .Owner }}/{{ .Repo.Name }}/issues/{{ .Issue.Number }}/assignees" class="shithub-assignee-form">
106121
           <input type="hidden" name="csrf_token" value="{{ .CSRFToken }}">
107122
           <input type="text" name="username" placeholder="username" required>
@@ -117,7 +132,7 @@
117132
           {{ $mid := .Issue.MilestoneID.Int64 }}
118133
           {{ range .Milestones }}{{ if eq .ID $mid }}<a href="/{{ $.Owner }}/{{ $.Repo.Name }}/milestones">{{ .Title }}</a>{{ end }}{{ end }}
119134
         {{ else }}<p class="shithub-muted">No milestone</p>{{ end }}
120
-        {{ if .Viewer.ID }}
135
+        {{ if .CanEditIssueMilestone }}
121136
         <form method="post" action="/{{ .Owner }}/{{ .Repo.Name }}/issues/{{ .Issue.Number }}/milestone">
122137
           <input type="hidden" name="csrf_token" value="{{ .CSRFToken }}">
123138
           <select name="milestone_id">
@@ -129,7 +144,7 @@
129144
         {{ end }}
130145
       </section>
131146
132
-      {{ if .Viewer.ID }}
147
+      {{ if .CanLockIssue }}
133148
       <section>
134149
         <h3>Lock</h3>
135150
         <form method="post" action="/{{ .Owner }}/{{ .Repo.Name }}/issues/{{ .Issue.Number }}/lock">
internal/web/templates/repo/labels.htmlmodified
@@ -8,7 +8,7 @@
88
     </h1>
99
   </header>
1010
11
-  {{ if .Viewer.ID }}
11
+  {{ if .CanManageIssue }}
1212
   <details class="shithub-label-create">
1313
     <summary class="shithub-button shithub-button-primary">New label</summary>
1414
     <form method="post" action="/{{ .Owner }}/{{ .Repo.Name }}/labels" class="shithub-label-form">
@@ -26,7 +26,7 @@
2626
     <li class="shithub-labels-row">
2727
       <span class="shithub-label" style="background-color: #{{ .Color }}">{{ .Name }}</span>
2828
       {{ if .Description }}<span class="shithub-muted">{{ .Description }}</span>{{ end }}
29
-      {{ if $.Viewer.ID }}
29
+      {{ if $.CanManageIssue }}
3030
       <details class="shithub-label-edit">
3131
         <summary class="shithub-button">Edit</summary>
3232
         <form method="post" action="/{{ $.Owner }}/{{ $.Repo.Name }}/labels/{{ .ID }}/update" class="shithub-label-form">
internal/web/templates/repo/milestones.htmlmodified
@@ -8,7 +8,7 @@
88
     </h1>
99
   </header>
1010
11
-  {{ if .Viewer.ID }}
11
+  {{ if .CanManageIssue }}
1212
   <details class="shithub-milestone-create">
1313
     <summary class="shithub-button shithub-button-primary">New milestone</summary>
1414
     <form method="post" action="/{{ .Owner }}/{{ .Repo.Name }}/milestones" class="shithub-milestone-form">
@@ -29,7 +29,7 @@
2929
         {{ if .DueOn.Valid }}<small>due {{ .DueOn.Time.Format "Jan 2, 2006" }}</small>{{ end }}
3030
       </h3>
3131
       {{ if .Description }}<p>{{ .Description }}</p>{{ end }}
32
-      {{ if $.Viewer.ID }}
32
+      {{ if $.CanManageIssue }}
3333
       <div class="shithub-milestone-actions">
3434
         <form method="post" action="/{{ $.Owner }}/{{ $.Repo.Name }}/milestones/{{ .ID }}/state">
3535
           <input type="hidden" name="csrf_token" value="{{ $.CSRFToken }}">
internal/web/templates/repo/pull_view.htmlmodified
@@ -96,7 +96,7 @@
9696
         </section>
9797
         {{ end }}
9898
99
-        {{ if .Viewer.ID }}
99
+        {{ if .CanReviewPull }}
100100
         <details class="shithub-pull-request-reviewer">
101101
           <summary class="shithub-button">Request review</summary>
102102
           <form method="post" action="/{{ .Owner }}/{{ .Repo.Name }}/pulls/{{ .PR.INumber }}/reviewers">
@@ -136,7 +136,8 @@
136136
           <p class="shithub-pull-state-{{ printf "%s" .PR.MergeableState }}">
137137
             {{ printf "%s" .PR.MergeableState }}
138138
           </p>
139
-          {{ if .Viewer.ID }}
139
+        {{ if or .CanMergePull .CanSetPullState .CanReadyPull }}
140
+            {{ if .CanMergePull }}
140141
             {{ if eq (printf "%s" .PR.MergeableState) "clean" }}
141142
               <form method="post" action="/{{ .Owner }}/{{ .Repo.Name }}/pulls/{{ .PR.INumber }}/merge" class="shithub-pull-merge-form">
142143
                 <input type="hidden" name="csrf_token" value="{{ .CSRFToken }}">
@@ -154,11 +155,14 @@
154155
             {{ else }}
155156
               <p class="shithub-muted">Mergeability is being computed…</p>
156157
             {{ end }}
158
+            {{ end }}
159
+            {{ if .CanSetPullState }}
157160
             <form method="post" action="/{{ .Owner }}/{{ .Repo.Name }}/pulls/{{ .PR.INumber }}/state" class="shithub-pull-state-form">
158161
               <input type="hidden" name="csrf_token" value="{{ .CSRFToken }}">
159162
               <button type="submit" name="state" value="closed" class="shithub-button">Close pull request</button>
160163
             </form>
161
-            {{ if .PR.Draft }}
164
+            {{ end }}
165
+            {{ if and .CanReadyPull .PR.Draft }}
162166
               <form method="post" action="/{{ .Owner }}/{{ .Repo.Name }}/pulls/{{ .PR.INumber }}/ready">
163167
                 <input type="hidden" name="csrf_token" value="{{ .CSRFToken }}">
164168
                 <button type="submit" class="shithub-button">Ready for review</button>
@@ -206,7 +210,7 @@
206210
               <div class="shithub-comment-body markdown-body">
207211
                 {{ if .C.BodyHtmlCached.Valid }}{{ safeHTML .C.BodyHtmlCached.String }}{{ else }}<p>{{ .C.Body }}</p>{{ end }}
208212
               </div>
209
-              {{ if $.Viewer.ID }}
213
+              {{ if $.CanReviewPull }}
210214
               <div class="shithub-pull-thread-actions">
211215
                 <form method="post" action="/{{ $.Owner }}/{{ $.Repo.Name }}/pulls/{{ $.PR.INumber }}/review-comments/{{ .C.ID }}/reply">
212216
                   <input type="hidden" name="csrf_token" value="{{ $.CSRFToken }}">
@@ -225,7 +229,7 @@
225229
         {{ end }}
226230
       {{ end }}
227231
228
-      {{ if .Viewer.ID }}
232
+      {{ if .CanReviewPull }}
229233
       <details class="shithub-pull-add-comment">
230234
         <summary class="shithub-button">Add inline comment</summary>
231235
         <form method="post" action="/{{ .Owner }}/{{ .Repo.Name }}/pulls/{{ .PR.INumber }}/review-comments">