`89bc712`

H1: author-self-close on issues and PRs via RepoRef.AuthorUserID

Authored by mfwolffe <wolffemf@dukes.jmu.edu> 5 days ago

SHA: 89bc712beb05a242c1901b4a118c4e597e356b7f
Parents: 6cb16b5
Tree: fdda0db

2 changed files

Status	File	+	-
M	`internal/auth/policy/resources.go`	15	0
M	`internal/web/handlers/repo/pulls.go`	31	14

internal/auth/policy/resources.gomodified

  	Visibility  string // "public" | "private"
  	IsArchived  bool
  	IsDeleted   bool
++
 +	// AuthorUserID is the author of the *issue or PR* being acted on,
 +	// when the action is one that grants author-self privileges
 +	// (`ActionIssueClose`, `ActionPullClose`). Zero means "no author
 +	// context" — e.g. a repo-level read or write — and is the default.
 +	// Handlers populate this only on the close paths; everywhere else
 +	// it stays zero.
 +	//
 +	// This is a pragmatic v1 shape: instead of introducing an
 +	// IssueRef/PullRef parallel to RepoRef, we widen RepoRef with the
 +	// one fact the close gate needs. When/if more issue-level rules
 +	// land (assignee privileges, labeler privileges) we'll graduate
 +	// to a proper IssueRef. The audit captured the design tradeoff;
 +	// don't add new fields here without re-reading that note.
 +	AuthorUserID int64
+ }
  // IsPublic returns true when the repo's visibility column is "public".

internal/web/handlers/repo/pulls.gomodified

  // MountPulls registers /{owner}/{repo}/pulls* routes. Reads are
  // public (subject to policy.Can(ActionPullRead)); writes require auth.
 -// The merge route enqueues a pr:merge worker job and renders a
 -// "merging…" page; the worker performs the actual git operation.
 +// The merge route runs synchronously inside the request: pulls.Merge
 +// performs the worktree operation, updates DB state, and the response
 +// redirects the user straight to the merged view. (An async-merge path
 +// can be reintroduced when very-large-repo merges become a real
 +// concern; the worker registration was deleted alongside the unused
 +// KindPRMerge in the audit remediation sprint.)
  func (h *Handlers) MountPulls(r chi.Router) {
  	r.Get("/{owner}/{repo}/pulls", h.pullsList)
  	r.Get("/{owner}/{repo}/pulls/{number}", h.pullView)
+ }
  func (h *Handlers) pullsDeps() pulls.Deps {
 -	return pulls.Deps{Pool: h.d.Pool, Logger: h.d.Logger}
 +	return pulls.Deps{Pool: h.d.Pool, Logger: h.d.Logger, Audit: h.d.Audit}
+ }
  // pullsList renders /{owner}/{repo}/pulls.
  	if err := json.Unmarshal(raw, &o); err != nil || o.Summary == "" {
  		return ""
+ 	}
 +	// Summary is bounded by the API's 256 KiB body cap (well under
 +	// markdown's 1 MiB ceiling). An error here only fires if a
 +	// structural precondition regresses; the function is a pure
 +	// presenter so we degrade to empty (the caller is just rendering
 +	// a tooltip-grade snippet on the PR checks panel).
  	html, _ := mdrender.RenderHTML([]byte(o.Summary))
  	return template.HTML(html) //nolint:gosec // sanitized by bluemonday UGCPolicy
+ }
  // pullSetState handles POST .../state.
  func (h *Handlers) pullSetState(w http.ResponseWriter, r *http.Request) {
 -	row, owner, ok := h.loadRepoAndAuthorize(w, r, policy.ActionPullClose)
 +	// Two-pass authorization: read access first, then ActionPullClose
 +	// with `repo.AuthorUserID = pr.AuthorUserID` set so the policy engine
 +	// grants author-self-close. Without the second pass, a non-collab
 +	// fork-PR author couldn't close their own PR (S00-S25 audit, H1).
 +	row, owner, ok := h.loadRepoAndAuthorize(w, r, policy.ActionPullRead)
  	if !ok {
  		return
+ 	}
  	if !ok {
  		return
+ 	}
 +	viewer := middleware.CurrentUserFromContext(r.Context())
 +	actor := policy.UserActor(viewer.ID, viewer.Username, viewer.IsSuspended, false)
 +	repoRef := policy.NewRepoRefFromRepo(row)
 +	if pr.IAuthorUserID.Valid {
 +		repoRef.AuthorUserID = pr.IAuthorUserID.Int64
 +	}
 +	if dec := policy.Can(r.Context(), policy.Deps{Pool: h.d.Pool}, actor, policy.ActionPullClose, repoRef); !dec.Allow {
 +		h.d.Render.HTTPError(w, r, policy.Maybe404(dec, repoRef, actor), "")
 +		return
 +	}
  	gitDir, err := h.d.RepoFS.RepoPath(owner.Username, row.Name)
  	if err != nil {
  		h.d.Render.HTTPError(w, r, http.StatusNotFound, "")
  		return
+ 	}
  	state := strings.TrimSpace(r.PostFormValue("state"))
 -	viewer := middleware.CurrentUserFromContext(r.Context())
  	if err := pulls.SetState(r.Context(), h.pullsDeps(), gitDir, viewer.ID, pr.IID, state); err != nil {
  		h.handlePullWriteError(w, r, err)
  		return
+ }
  // pullMerge handles POST .../merge. Performs the merge synchronously
 -// (so the redirect lands on the merged state). Heavy merges could be
 -// async via the pr:merge job; v1 is synchronous so the user sees an
 -// immediate result on small merges.
 +// inside the request so the redirect lands on the merged state. The
 +// pulls.Merge orchestrator updates repos.default_branch_oid in the
 +// same tx when the base IS the default branch, since update-ref
 +// bypasses the push:process hook that normally maintains the column.
  func (h *Handlers) pullMerge(w http.ResponseWriter, r *http.Request) {
  	row, owner, ok := h.loadRepoAndAuthorize(w, r, policy.ActionPullMerge)
  	if !ok {
  		h.handlePullWriteError(w, r, err)
  		return
+ 	}
 -	// After merge, push:process won't fire (the update-ref bypassed
 -	// the hook). Trigger a default-branch-OID refresh manually.
 -	go func() {
 -		// Fire-and-forget; the user is already redirected.
 -		// Failure is logged but doesn't affect UX.
 -	}()
  	h.redirectPull(w, r, owner.Username, row.Name, pr.INumber)
+ }