neiam /gitgud
Git Gud
public · Issues · Pulls · Labels · Forks · Compare · Actions success · Packages
⭐
Log in to mark this repository.
Add repo- and org-scoped bans, and scope reports to their owner
7accb73 · Gabriel Morell · 2026-09-10 16:08
Message
{commit_body(@commit)}
Files changed
modified
lib/git_gud/issues.ex
+27
−2
@@ -154,7 +154,15 @@ defmodule GitGud.Issues do
| 154 | 154 | @doc """ |
| 155 | 155 | Open a new issue. Allocates the next per-repo number atomically. |
| 156 | 156 | """ |
| 157 | − def create_issue(%Repository{id: rid} = repo, %User{id: uid} = author, attrs) do | |
| 157 | + def create_issue(%Repository{} = repo, %User{} = author, attrs) do | |
| 158 | + if GitGud.Moderation.banned?(repo, author) do | |
| 159 | + {:error, :banned} | |
| 160 | + else | |
| 161 | + do_create_issue(repo, author, attrs) | |
| 162 | + end | |
| 163 | + end | |
| 164 | + | |
| 165 | + defp do_create_issue(%Repository{id: rid} = repo, %User{id: uid} = author, attrs) do | |
| 158 | 166 | result = |
| 159 | 167 | Repo.transaction(fn -> |
| 160 | 168 | next = Numbering.next_for(rid) |
@@ -187,7 +195,24 @@ defmodule GitGud.Issues do
| 187 | 195 | |> Repo.update() |
| 188 | 196 | end |
| 189 | 197 | |
| 190 | − def add_comment(%Issue{} = issue, %User{id: uid} = author, attrs) do | |
| 198 | + def add_comment(%Issue{} = issue, %User{} = author, attrs) do | |
| 199 | + if banned_from?(issue, author) do | |
| 200 | + {:error, :banned} | |
| 201 | + else | |
| 202 | + do_add_comment(issue, author, attrs) | |
| 203 | + end | |
| 204 | + end | |
| 205 | + | |
| 206 | + # The issue carries only a repository_id, so the repo is fetched to | |
| 207 | + # ask about the ban — an org-wide ban is decided by the repo's owner. | |
| 208 | + defp banned_from?(%Issue{repository_id: rid}, author) do | |
| 209 | + case Repo.get(Repository, rid) do | |
| 210 | + nil -> false | |
| 211 | + repo -> GitGud.Moderation.banned?(repo, author) | |
| 212 | + end | |
| 213 | + end | |
| 214 | + | |
| 215 | + defp do_add_comment(%Issue{} = issue, %User{id: uid} = author, attrs) do | |
| 191 | 216 | case %IssueComment{issue_id: issue.id, author_id: uid} |
| 192 | 217 | |> IssueComment.changeset(attrs) |
| 193 | 218 | |> Repo.insert() do |
added
lib/git_gud/moderation.ex
+149
−0
@@ -0,0 +1,149 @@
| 1 | +defmodule GitGud.Moderation do | |
| 2 | + @moduledoc """ | |
| 3 | + Repo- and org-scoped bans on local users. | |
| 4 | + | |
| 5 | + A repo admin can stop someone acting in their repo; an org admin can | |
| 6 | + stop them across every repo the org owns. Neither reaches further | |
| 7 | + than that — suspending an account instance-wide stays an instance | |
| 8 | + admin's power. | |
| 9 | + | |
| 10 | + Bans stop *writing*, not reading. Visibility already decides who can | |
| 11 | + see a repo, and a read block on a public one is unenforceable anyway | |
| 12 | + — the same person can log out. What a ban does is prevent opening | |
| 13 | + issues and PRs, commenting, reviewing, and requesting reviews, and it | |
| 14 | + hides the content they already wrote in that scope. | |
| 15 | + | |
| 16 | + Expiry needs no sweeper: `banned?/2` filters on `expires_at` in the | |
| 17 | + query, so a lapsed ban simply stops matching. The row stays as a | |
| 18 | + record of what happened. | |
| 19 | + """ | |
| 20 | + | |
| 21 | + import Ecto.Query, warn: false | |
| 22 | + | |
| 23 | + alias GitGud.Accounts.User | |
| 24 | + alias GitGud.Moderation.Ban | |
| 25 | + alias GitGud.Organizations.Organization | |
| 26 | + alias GitGud.Repo | |
| 27 | + alias GitGud.Repositories.Repository | |
| 28 | + | |
| 29 | + @doc """ | |
| 30 | + Ban `user` from a repository or an organization. | |
| 31 | + | |
| 32 | + `scope` is a `%Repository{}` or an `%Organization{}`. Options: | |
| 33 | + `:reason`, `:expires_at`, `:banned_by`. | |
| 34 | + """ | |
| 35 | + def ban(scope, %User{} = user, opts \\ []) do | |
| 36 | + attrs = | |
| 37 | + scope | |
| 38 | + |> scope_attrs() | |
| 39 | + |> Map.merge(%{ | |
| 40 | + banned_user_id: user.id, | |
| 41 | + banned_by_id: opts[:banned_by] && opts[:banned_by].id, | |
| 42 | + reason: opts[:reason], | |
| 43 | + expires_at: opts[:expires_at] | |
| 44 | + }) | |
| 45 | + | |
| 46 | + %Ban{} |> Ban.changeset(attrs) |> Repo.insert() | |
| 47 | + end | |
| 48 | + | |
| 49 | + defp scope_attrs(%Repository{id: id}), do: %{repository_id: id} | |
| 50 | + defp scope_attrs(%Organization{id: id}), do: %{organization_id: id} | |
| 51 | + | |
| 52 | + @doc "Lift a ban. Keeps the row, stamped with who lifted it and when." | |
| 53 | + def lift(%Ban{} = ban, %User{} = actor) do | |
| 54 | + ban |> Ban.lift_changeset(actor) |> Repo.update() | |
| 55 | + end | |
| 56 | + | |
| 57 | + def get_ban(id), do: Repo.get(Ban, id) |> Repo.preload([:banned_user, :banned_by]) | |
| 58 | + | |
| 59 | + @doc """ | |
| 60 | + Whether `user` is currently banned from writing in `repo`. | |
| 61 | + | |
| 62 | + Checks the repo's own bans and those of the org that owns it — an org | |
| 63 | + ban covers everything the org owns, so a repo admin can't | |
| 64 | + accidentally undercut it by not knowing about it. | |
| 65 | + | |
| 66 | + `nil` (anonymous) is never banned; whether they can act at all is a | |
| 67 | + separate question that authentication already answers. | |
| 68 | + """ | |
| 69 | + def banned?(_repo, nil), do: false | |
| 70 | + | |
| 71 | + def banned?(%Repository{} = repo, %User{id: uid}) do | |
| 72 | + Ban | |
| 73 | + |> where([b], b.banned_user_id == ^uid) | |
| 74 | + |> in_force() | |
| 75 | + |> covering(repo) | |
| 76 | + |> Repo.exists?() | |
| 77 | + end | |
| 78 | + | |
| 79 | + # A ban still standing: not lifted, and not lapsed. Expiry is a query | |
| 80 | + # condition rather than a swept flag, so nothing has to run for a | |
| 81 | + # ban to end. | |
| 82 | + defp in_force(query) do | |
| 83 | + now = DateTime.utc_now() | |
| 84 | + | |
| 85 | + query | |
| 86 | + |> where([b], is_nil(b.lifted_at)) | |
| 87 | + |> where([b], is_nil(b.expires_at) or b.expires_at > ^now) | |
| 88 | + end | |
| 89 | + | |
| 90 | + # Bans that reach into this repo: its own, plus its owning org's. | |
| 91 | + # Split on whether there *is* an owning org — a user-owned repo has a | |
| 92 | + # nil organization_id, and Ecto rightly refuses `== nil` in a query. | |
| 93 | + defp covering(query, %Repository{id: rid, organization_id: nil}), | |
| 94 | + do: where(query, [b], b.repository_id == ^rid) | |
| 95 | + | |
| 96 | + defp covering(query, %Repository{id: rid, organization_id: oid}), | |
| 97 | + do: where(query, [b], b.repository_id == ^rid or b.organization_id == ^oid) | |
| 98 | + | |
| 99 | + @doc """ | |
| 100 | + The ban in force for `user` in `repo`, if any — so a page can say | |
| 101 | + why, and whether it lapses. | |
| 102 | + | |
| 103 | + An org-wide ban takes precedence over a repo one: it's the broader | |
| 104 | + statement, and lifting the repo ban wouldn't change anything. | |
| 105 | + """ | |
| 106 | + def active_ban(%Repository{} = repo, %User{id: uid}) do | |
| 107 | + Ban | |
| 108 | + |> where([b], b.banned_user_id == ^uid) | |
| 109 | + |> in_force() | |
| 110 | + |> covering(repo) | |
| 111 | + # Non-null organization_id sorts first, so an org ban is reported | |
| 112 | + # ahead of a repo one. | |
| 113 | + |> order_by([b], desc_nulls_last: b.organization_id) | |
| 114 | + |> limit(1) | |
| 115 | + |> preload([:banned_by, :organization, :repository]) | |
| 116 | + |> Repo.one() | |
| 117 | + end | |
| 118 | + | |
| 119 | + def active_ban(_repo, nil), do: nil | |
| 120 | + | |
| 121 | + @doc "Bans in a scope, newest first. Lifted and lapsed ones included." | |
| 122 | + def list_bans(%Repository{id: id}), do: bans_where(repository_id: id) | |
| 123 | + def list_bans(%Organization{id: id}), do: bans_where(organization_id: id) | |
| 124 | + | |
| 125 | + defp bans_where(clause) do | |
| 126 | + Ban | |
| 127 | + |> where(^clause) | |
| 128 | + |> order_by([b], desc: b.id) | |
| 129 | + |> preload([:banned_user, :banned_by, :lifted_by]) | |
| 130 | + |> Repo.all() | |
| 131 | + end | |
| 132 | + | |
| 133 | + @doc "Ids of users currently banned in `repo`, for hiding their content." | |
| 134 | + def banned_user_ids(%Repository{} = repo) do | |
| 135 | + Ban | |
| 136 | + |> in_force() | |
| 137 | + |> covering(repo) | |
| 138 | + |> select([b], b.banned_user_id) | |
| 139 | + |> distinct(true) | |
| 140 | + |> Repo.all() | |
| 141 | + end | |
| 142 | + | |
| 143 | + @doc "Count of bans currently in force in a scope — for a settings badge." | |
| 144 | + def active_ban_count(scope) do | |
| 145 | + scope | |
| 146 | + |> list_bans() | |
| 147 | + |> Enum.count(&Ban.active?/1) | |
| 148 | + end | |
| 149 | +end |
added
lib/git_gud/moderation/ban.ex
+91
−0
@@ -0,0 +1,91 @@
| 1 | +defmodule GitGud.Moderation.Ban do | |
| 2 | + use Ecto.Schema | |
| 3 | + import Ecto.Changeset | |
| 4 | + | |
| 5 | + alias GitGud.Accounts.User | |
| 6 | + alias GitGud.Organizations.Organization | |
| 7 | + alias GitGud.Repositories.Repository | |
| 8 | + | |
| 9 | + schema "moderation_bans" do | |
| 10 | + field :reason, :string | |
| 11 | + field :expires_at, :utc_datetime | |
| 12 | + field :lifted_at, :utc_datetime | |
| 13 | + | |
| 14 | + belongs_to :organization, Organization | |
| 15 | + belongs_to :repository, Repository | |
| 16 | + belongs_to :banned_user, User | |
| 17 | + belongs_to :banned_by, User | |
| 18 | + belongs_to :lifted_by, User | |
| 19 | + | |
| 20 | + timestamps(type: :utc_datetime) | |
| 21 | + end | |
| 22 | + | |
| 23 | + def changeset(ban, attrs) do | |
| 24 | + ban | |
| 25 | + |> cast(attrs, [ | |
| 26 | + :organization_id, | |
| 27 | + :repository_id, | |
| 28 | + :banned_user_id, | |
| 29 | + :banned_by_id, | |
| 30 | + :reason, | |
| 31 | + :expires_at | |
| 32 | + ]) | |
| 33 | + |> validate_required([:banned_user_id]) | |
| 34 | + |> validate_one_scope() | |
| 35 | + |> validate_length(:reason, max: 500) | |
| 36 | + |> validate_future_expiry() | |
| 37 | + |> unique_constraint(:banned_user_id, | |
| 38 | + name: :moderation_bans_active_org_index, | |
| 39 | + message: "is already banned from this organization" | |
| 40 | + ) | |
| 41 | + |> unique_constraint(:banned_user_id, | |
| 42 | + name: :moderation_bans_active_repo_index, | |
| 43 | + message: "is already banned from this repository" | |
| 44 | + ) | |
| 45 | + |> check_constraint(:organization_id, | |
| 46 | + name: :moderation_bans_one_scope, | |
| 47 | + message: "must name exactly one of an organization or a repository" | |
| 48 | + ) | |
| 49 | + end | |
| 50 | + | |
| 51 | + defp validate_one_scope(changeset) do | |
| 52 | + org = get_field(changeset, :organization_id) | |
| 53 | + repo = get_field(changeset, :repository_id) | |
| 54 | + | |
| 55 | + case {org, repo} do | |
| 56 | + {nil, nil} -> | |
| 57 | + add_error(changeset, :organization_id, "a ban needs a scope") | |
| 58 | + | |
| 59 | + {o, r} when not is_nil(o) and not is_nil(r) -> | |
| 60 | + add_error(changeset, :organization_id, "a ban has one scope") | |
| 61 | + | |
| 62 | + _ -> | |
| 63 | + changeset | |
| 64 | + end | |
| 65 | + end | |
| 66 | + | |
| 67 | + # An expiry in the past would be a ban that never applies, which is | |
| 68 | + # more likely a mistake than an intention. | |
| 69 | + defp validate_future_expiry(changeset) do | |
| 70 | + case get_field(changeset, :expires_at) do | |
| 71 | + nil -> | |
| 72 | + changeset | |
| 73 | + | |
| 74 | + at -> | |
| 75 | + if DateTime.compare(at, DateTime.utc_now()) == :gt, | |
| 76 | + do: changeset, | |
| 77 | + else: add_error(changeset, :expires_at, "must be in the future") | |
| 78 | + end | |
| 79 | + end | |
| 80 | + | |
| 81 | + def lift_changeset(ban, %User{id: uid}) do | |
| 82 | + change(ban, lifted_at: DateTime.utc_now(:second), lifted_by_id: uid) | |
| 83 | + end | |
| 84 | + | |
| 85 | + @doc "Whether this ban is in force right now." | |
| 86 | + def active?(%__MODULE__{lifted_at: lifted}) when not is_nil(lifted), do: false | |
| 87 | + def active?(%__MODULE__{expires_at: nil}), do: true | |
| 88 | + | |
| 89 | + def active?(%__MODULE__{expires_at: at}), | |
| 90 | + do: DateTime.compare(at, DateTime.utc_now()) == :gt | |
| 91 | +end |
modified
lib/git_gud/pull_requests.ex
+43
−2
@@ -174,7 +174,15 @@ defmodule GitGud.PullRequests do
| 174 | 174 | Open a new PR. Resolves source/target to SHAs, computes the merge base, |
| 175 | 175 | and snapshots both head and base into the PR row. |
| 176 | 176 | """ |
| 177 | − def create_pull_request(%Repository{} = repo, %User{id: uid} = author, attrs) do | |
| 177 | + def create_pull_request(%Repository{} = repo, %User{} = author, attrs) do | |
| 178 | + if GitGud.Moderation.banned?(repo, author) do | |
| 179 | + {:error, :banned} | |
| 180 | + else | |
| 181 | + do_create_pull_request(repo, author, attrs) | |
| 182 | + end | |
| 183 | + end | |
| 184 | + | |
| 185 | + defp do_create_pull_request(%Repository{} = repo, %User{id: uid} = author, attrs) do | |
| 178 | 186 | source_ref = attrs["source_ref"] || attrs[:source_ref] |
| 179 | 187 | target_ref = attrs["target_ref"] || attrs[:target_ref] |
| 180 | 188 | source_repo_id = attrs["source_repository_id"] || attrs[:source_repository_id] |
@@ -672,7 +680,24 @@ defmodule GitGud.PullRequests do
| 672 | 680 | |
| 673 | 681 | # ── comments + reviews ─────────────────────────────────────────────── |
| 674 | 682 | |
| 675 | − def add_comment(%PullRequest{} = pr, %User{id: uid} = author, attrs) do | |
| 683 | + def add_comment(%PullRequest{} = pr, %User{} = author, attrs) do | |
| 684 | + if banned_from?(pr, author) do | |
| 685 | + {:error, :banned} | |
| 686 | + else | |
| 687 | + do_add_comment(pr, author, attrs) | |
| 688 | + end | |
| 689 | + end | |
| 690 | + | |
| 691 | + # A PR knows its repository_id; the repo decides, since an org ban | |
| 692 | + # covers everything the org owns. | |
| 693 | + defp banned_from?(%PullRequest{repository_id: rid}, author) do | |
| 694 | + case Repo.get(Repository, rid) do | |
| 695 | + nil -> false | |
| 696 | + repo -> GitGud.Moderation.banned?(repo, author) | |
| 697 | + end | |
| 698 | + end | |
| 699 | + | |
| 700 | + defp do_add_comment(%PullRequest{} = pr, %User{id: uid} = author, attrs) do | |
| 676 | 701 | case %PrComment{pull_request_id: pr.id, author_id: uid} |
| 677 | 702 | |> PrComment.changeset(attrs) |
| 678 | 703 | |> Repo.insert() do |
@@ -689,6 +714,14 @@ defmodule GitGud.PullRequests do
| 689 | 714 | def add_review(%PullRequest{} = pr, %User{id: uid} = reviewer, attrs) do |
| 690 | 715 | changeset = PrReview.changeset(%PrReview{pull_request_id: pr.id, reviewer_id: uid}, attrs) |
| 691 | 716 | |
| 717 | + if banned_from?(pr, reviewer) do | |
| 718 | + {:error, :banned} | |
| 719 | + else | |
| 720 | + add_review_checked(pr, reviewer, changeset, uid) | |
| 721 | + end | |
| 722 | + end | |
| 723 | + | |
| 724 | + defp add_review_checked(pr, reviewer, changeset, uid) do | |
| 692 | 725 | if self_approval_blocked?(pr, uid, changeset) do |
| 693 | 726 | {:error, :self_approval} |
| 694 | 727 | else |
@@ -931,6 +964,14 @@ defmodule GitGud.PullRequests do
| 931 | 964 | do: {:error, :self_request} |
| 932 | 965 | |
| 933 | 966 | def request_review(%PullRequest{} = pr, %User{} = reviewer, requested_by) do |
| 967 | + cond do | |
| 968 | + banned_from?(pr, reviewer) -> {:error, :banned} | |
| 969 | + requested_by && banned_from?(pr, requested_by) -> {:error, :banned} | |
| 970 | + true -> insert_review_request(pr, reviewer, requested_by) | |
| 971 | + end | |
| 972 | + end | |
| 973 | + | |
| 974 | + defp insert_review_request(%PullRequest{} = pr, %User{} = reviewer, requested_by) do | |
| 934 | 975 | attrs = %{ |
| 935 | 976 | pull_request_id: pr.id, |
| 936 | 977 | reviewer_id: reviewer.id, |
modified
lib/git_gud/reports.ex
+102
−1
@@ -55,18 +55,119 @@ defmodule GitGud.Reports do
| 55 | 55 | if target_already_moderated?(target_type, target_id) do |
| 56 | 56 | {:error, :already_moderated} |
| 57 | 57 | else |
| 58 | + {repository_id, organization_id} = owner_scope(target_type, target_id) | |
| 59 | + | |
| 58 | 60 | %Report{} |
| 59 | 61 | |> Report.open_changeset(%{ |
| 60 | 62 | reporter_id: uid, |
| 61 | 63 | target_type: target_type, |
| 62 | 64 | target_id: target_id, |
| 63 | 65 | reason: reason, |
| 64 | − details: details | |
| 66 | + details: details, | |
| 67 | + repository_id: repository_id, | |
| 68 | + organization_id: organization_id | |
| 65 | 69 | }) |
| 66 | 70 | |> Repo.insert() |
| 67 | 71 | end |
| 68 | 72 | end |
| 69 | 73 | |
| 74 | + @doc """ | |
| 75 | + The repo and org a report belongs to, resolved from its target. | |
| 76 | + | |
| 77 | + Federation targets (`actor`, `inbox_delivery`) belong to neither and | |
| 78 | + come back `{nil, nil}` — those stay instance-admin business. | |
| 79 | + """ | |
| 80 | + def owner_scope(target_type, target_id) do | |
| 81 | + case repository_of(target_type, target_id) do | |
| 82 | + nil -> {nil, nil} | |
| 83 | + repo -> {repo.id, repo.organization_id} | |
| 84 | + end | |
| 85 | + end | |
| 86 | + | |
| 87 | + defp repository_of("issue", id) do | |
| 88 | + Repo.one( | |
| 89 | + from i in GitGud.Issues.Issue, | |
| 90 | + where: i.id == ^id, | |
| 91 | + join: r in assoc(i, :repository), | |
| 92 | + select: r | |
| 93 | + ) | |
| 94 | + end | |
| 95 | + | |
| 96 | + defp repository_of("issue_comment", id) do | |
| 97 | + Repo.one( | |
| 98 | + from c in GitGud.Issues.IssueComment, | |
| 99 | + where: c.id == ^id, | |
| 100 | + join: i in assoc(c, :issue), | |
| 101 | + join: r in assoc(i, :repository), | |
| 102 | + select: r | |
| 103 | + ) | |
| 104 | + end | |
| 105 | + | |
| 106 | + defp repository_of("pull_request", id) do | |
| 107 | + Repo.one( | |
| 108 | + from p in GitGud.PullRequests.PullRequest, | |
| 109 | + where: p.id == ^id, | |
| 110 | + join: r in assoc(p, :repository), | |
| 111 | + select: r | |
| 112 | + ) | |
| 113 | + end | |
| 114 | + | |
| 115 | + defp repository_of("pr_comment", id) do | |
| 116 | + Repo.one( | |
| 117 | + from c in GitGud.PullRequests.PrComment, | |
| 118 | + where: c.id == ^id, | |
| 119 | + join: p in assoc(c, :pull_request), | |
| 120 | + join: r in assoc(p, :repository), | |
| 121 | + select: r | |
| 122 | + ) | |
| 123 | + end | |
| 124 | + | |
| 125 | + defp repository_of(_type, _id), do: nil | |
| 126 | + | |
| 127 | + @doc "Open reports about content in `repo`." | |
| 128 | + def list_open_for_repo(%GitGud.Repositories.Repository{id: rid}, opts \\ []) do | |
| 129 | + limit = Keyword.get(opts, :limit, 100) | |
| 130 | + | |
| 131 | + Report | |
| 132 | + |> where([r], r.state == "open" and r.repository_id == ^rid) | |
| 133 | + |> order_by([r], desc: r.inserted_at) | |
| 134 | + |> limit(^limit) | |
| 135 | + |> preload([:reporter]) | |
| 136 | + |> Repo.all() | |
| 137 | + end | |
| 138 | + | |
| 139 | + @doc """ | |
| 140 | + Open reports about content in any repo `org` owns. | |
| 141 | + | |
| 142 | + Scoped by the denormalised `organization_id`, so a repo that moves | |
| 143 | + between owners takes its future reports with it. | |
| 144 | + """ | |
| 145 | + def list_open_for_org(%GitGud.Organizations.Organization{id: oid}, opts \\ []) do | |
| 146 | + limit = Keyword.get(opts, :limit, 100) | |
| 147 | + | |
| 148 | + Report | |
| 149 | + |> where([r], r.state == "open" and r.organization_id == ^oid) | |
| 150 | + |> order_by([r], desc: r.inserted_at) | |
| 151 | + |> limit(^limit) | |
| 152 | + |> preload([:reporter, :repository]) | |
| 153 | + |> Repo.all() | |
| 154 | + end | |
| 155 | + | |
| 156 | + @doc "How many open reports sit in this scope — for a settings badge." | |
| 157 | + def open_count_for_repo(%GitGud.Repositories.Repository{id: rid}), | |
| 158 | + do: | |
| 159 | + Repo.aggregate( | |
| 160 | + from(r in Report, where: r.state == "open" and r.repository_id == ^rid), | |
| 161 | + :count | |
| 162 | + ) | |
| 163 | + | |
| 164 | + def open_count_for_org(%GitGud.Organizations.Organization{id: oid}), | |
| 165 | + do: | |
| 166 | + Repo.aggregate( | |
| 167 | + from(r in Report, where: r.state == "open" and r.organization_id == ^oid), | |
| 168 | + :count | |
| 169 | + ) | |
| 170 | + | |
| 70 | 171 | defp target_already_moderated?(type, id) do |
| 71 | 172 | case target_schema(type) do |
| 72 | 173 | {:ok, schema} -> |
modified
lib/git_gud/reports/report.ex
+14
−1
@@ -19,13 +19,26 @@ defmodule GitGud.Reports.Report do
| 19 | 19 | |
| 20 | 20 | belongs_to :reporter, User |
| 21 | 21 | belongs_to :resolved_by, User |
| 22 | + # Denormalised from the target so an owner-scoped queue is one | |
| 23 | + # query rather than a join per target type. Null for federation | |
| 24 | + # reports, which belong to instance admins. | |
| 25 | + belongs_to :repository, GitGud.Repositories.Repository | |
| 26 | + belongs_to :organization, GitGud.Organizations.Organization | |
| 22 | 27 | |
| 23 | 28 | timestamps(type: :utc_datetime) |
| 24 | 29 | end |
| 25 | 30 | |
| 26 | 31 | def open_changeset(report, attrs) do |
| 27 | 32 | report |
| 28 | − |> cast(attrs, [:reporter_id, :target_type, :target_id, :reason, :details]) | |
| 33 | + |> cast(attrs, [ | |
| 34 | + :reporter_id, | |
| 35 | + :target_type, | |
| 36 | + :target_id, | |
| 37 | + :reason, | |
| 38 | + :details, | |
| 39 | + :repository_id, | |
| 40 | + :organization_id | |
| 41 | + ]) | |
| 29 | 42 | |> validate_required([:target_type, :target_id, :reason]) |
| 30 | 43 | |> validate_inclusion(:target_type, @valid_target_types) |
| 31 | 44 | |> validate_inclusion(:reason, @valid_reasons) |
added
priv/repo/migrations/20260910060000_scope_reports_to_owner.exs
+63
−0
@@ -0,0 +1,63 @@
| 1 | +defmodule GitGud.Repo.Migrations.ScopeReportsToOwner do | |
| 2 | + use Ecto.Migration | |
| 3 | + | |
| 4 | + # Reports know only `(target_type, target_id)`, so asking "what has | |
| 5 | + # been reported in my repo?" meant a different join per target type on | |
| 6 | + # every page load. Denormalise the owning repo and org onto the row | |
| 7 | + # instead, filled in at report time. | |
| 8 | + # | |
| 9 | + # Both stay nullable: `actor` and `inbox_delivery` reports are about | |
| 10 | + # federation rather than a repo, and belong to instance admins alone. | |
| 11 | + def up do | |
| 12 | + alter table(:reports) do | |
| 13 | + add :repository_id, references(:repositories, on_delete: :nilify_all) | |
| 14 | + add :organization_id, references(:organizations, on_delete: :nilify_all) | |
| 15 | + end | |
| 16 | + | |
| 17 | + create index(:reports, [:repository_id, :state]) | |
| 18 | + create index(:reports, [:organization_id, :state]) | |
| 19 | + | |
| 20 | + flush() | |
| 21 | + | |
| 22 | + execute """ | |
| 23 | + UPDATE reports r SET repository_id = i.repository_id | |
| 24 | + FROM issues i | |
| 25 | + WHERE r.target_type = 'issue' AND r.target_id = i.id | |
| 26 | + """ | |
| 27 | + | |
| 28 | + execute """ | |
| 29 | + UPDATE reports r SET repository_id = i.repository_id | |
| 30 | + FROM issue_comments c JOIN issues i ON i.id = c.issue_id | |
| 31 | + WHERE r.target_type = 'issue_comment' AND r.target_id = c.id | |
| 32 | + """ | |
| 33 | + | |
| 34 | + execute """ | |
| 35 | + UPDATE reports r SET repository_id = p.repository_id | |
| 36 | + FROM pull_requests p | |
| 37 | + WHERE r.target_type = 'pull_request' AND r.target_id = p.id | |
| 38 | + """ | |
| 39 | + | |
| 40 | + execute """ | |
| 41 | + UPDATE reports r SET repository_id = p.repository_id | |
| 42 | + FROM pr_comments c JOIN pull_requests p ON p.id = c.pull_request_id | |
| 43 | + WHERE r.target_type = 'pr_comment' AND r.target_id = c.id | |
| 44 | + """ | |
| 45 | + | |
| 46 | + # The org follows from the repo for anything org-owned. | |
| 47 | + execute """ | |
| 48 | + UPDATE reports r SET organization_id = repo.organization_id | |
| 49 | + FROM repositories repo | |
| 50 | + WHERE r.repository_id = repo.id AND repo.organization_id IS NOT NULL | |
| 51 | + """ | |
| 52 | + end | |
| 53 | + | |
| 54 | + def down do | |
| 55 | + drop index(:reports, [:repository_id, :state]) | |
| 56 | + drop index(:reports, [:organization_id, :state]) | |
| 57 | + | |
| 58 | + alter table(:reports) do | |
| 59 | + remove :repository_id | |
| 60 | + remove :organization_id | |
| 61 | + end | |
| 62 | + end | |
| 63 | +end |
added
priv/repo/migrations/20260910070000_create_moderation_bans.exs
+46
−0
@@ -0,0 +1,46 @@
| 1 | +defmodule GitGud.Repo.Migrations.CreateModerationBans do | |
| 2 | + use Ecto.Migration | |
| 3 | + | |
| 4 | + # Stopping a local user from acting in one repo, or across every repo | |
| 5 | + # an org owns. | |
| 6 | + # | |
| 7 | + # Nothing here suspends an account instance-wide — that stays an | |
| 8 | + # instance-admin power. This is scoped so a repo or org admin can | |
| 9 | + # protect their own ground without reaching further than it. | |
| 10 | + def change do | |
| 11 | + create table(:moderation_bans) do | |
| 12 | + add :organization_id, references(:organizations, on_delete: :delete_all) | |
| 13 | + add :repository_id, references(:repositories, on_delete: :delete_all) | |
| 14 | + | |
| 15 | + add :banned_user_id, references(:users, on_delete: :delete_all), null: false | |
| 16 | + add :banned_by_id, references(:users, on_delete: :nilify_all) | |
| 17 | + | |
| 18 | + add :reason, :string | |
| 19 | + # Null means it lasts until someone lifts it. | |
| 20 | + add :expires_at, :utc_datetime | |
| 21 | + add :lifted_at, :utc_datetime | |
| 22 | + add :lifted_by_id, references(:users, on_delete: :nilify_all) | |
| 23 | + | |
| 24 | + timestamps(type: :utc_datetime) | |
| 25 | + end | |
| 26 | + | |
| 27 | + # Exactly one scope, never both and never neither. | |
| 28 | + create constraint(:moderation_bans, :moderation_bans_one_scope, | |
| 29 | + check: "(organization_id IS NULL) <> (repository_id IS NULL)" | |
| 30 | + ) | |
| 31 | + | |
| 32 | + # One live ban per person per scope; lifted ones are kept as record. | |
| 33 | + create unique_index(:moderation_bans, [:organization_id, :banned_user_id], | |
| 34 | + where: "lifted_at IS NULL AND organization_id IS NOT NULL", | |
| 35 | + name: :moderation_bans_active_org_index | |
| 36 | + ) | |
| 37 | + | |
| 38 | + create unique_index(:moderation_bans, [:repository_id, :banned_user_id], | |
| 39 | + where: "lifted_at IS NULL AND repository_id IS NOT NULL", | |
| 40 | + name: :moderation_bans_active_repo_index | |
| 41 | + ) | |
| 42 | + | |
| 43 | + # "Is this person banned here?" — checked on every write. | |
| 44 | + create index(:moderation_bans, [:banned_user_id, :lifted_at]) | |
| 45 | + end | |
| 46 | +end |
added
test/git_gud/moderation_test.exs
+238
−0
@@ -0,0 +1,238 @@
| 1 | +defmodule GitGud.ModerationTest do | |
| 2 | + use GitGud.DataCase, async: false | |
| 3 | + | |
| 4 | + import GitGud.AccountsFixtures | |
| 5 | + import GitGud.ForgeFixtures | |
| 6 | + | |
| 7 | + alias GitGud.Issues | |
| 8 | + alias GitGud.Moderation | |
| 9 | + alias GitGud.Organizations | |
| 10 | + alias GitGud.PullRequests | |
| 11 | + alias GitGud.Repositories | |
| 12 | + | |
| 13 | + defp org_repo(handle) do | |
| 14 | + admin = user_fixture() | |
| 15 | + {:ok, org} = Organizations.create_organization(admin, %{"handle" => handle}) | |
| 16 | + | |
| 17 | + {:ok, repo} = | |
| 18 | + Repositories.create_repository_for_org(org, admin, %{ | |
| 19 | + "name" => "repo-#{System.unique_integer([:positive])}", | |
| 20 | + "visibility" => "public" | |
| 21 | + }) | |
| 22 | + | |
| 23 | + {admin, org, repo} | |
| 24 | + end | |
| 25 | + | |
| 26 | + describe "scope" do | |
| 27 | + test "a repo ban applies to that repo" do | |
| 28 | + {_owner, repo} = repository_fixture() | |
| 29 | + nuisance = user_fixture() | |
| 30 | + | |
| 31 | + {:ok, _ban} = Moderation.ban(repo, nuisance) | |
| 32 | + | |
| 33 | + assert Moderation.banned?(repo, nuisance) | |
| 34 | + end | |
| 35 | + | |
| 36 | + test "a repo ban doesn't leak to another repo" do | |
| 37 | + {_owner, repo} = repository_fixture() | |
| 38 | + {_other_owner, other} = repository_fixture() | |
| 39 | + nuisance = user_fixture() | |
| 40 | + | |
| 41 | + {:ok, _ban} = Moderation.ban(repo, nuisance) | |
| 42 | + | |
| 43 | + refute Moderation.banned?(other, nuisance) | |
| 44 | + end | |
| 45 | + | |
| 46 | + test "an org ban covers every repo the org owns" do | |
| 47 | + {_admin, org, repo} = org_repo("mod-org-wide") | |
| 48 | + nuisance = user_fixture() | |
| 49 | + | |
| 50 | + {:ok, _ban} = Moderation.ban(org, nuisance) | |
| 51 | + | |
| 52 | + assert Moderation.banned?(repo, nuisance) | |
| 53 | + end | |
| 54 | + | |
| 55 | + test "an org ban covers repos added after it" do | |
| 56 | + {admin, org, _repo} = org_repo("mod-org-later") | |
| 57 | + nuisance = user_fixture() | |
| 58 | + {:ok, _ban} = Moderation.ban(org, nuisance) | |
| 59 | + | |
| 60 | + {:ok, later} = | |
| 61 | + Repositories.create_repository_for_org(org, admin, %{ | |
| 62 | + "name" => "added-later", | |
| 63 | + "visibility" => "public" | |
| 64 | + }) | |
| 65 | + | |
| 66 | + assert Moderation.banned?(later, nuisance) | |
| 67 | + end | |
| 68 | + | |
| 69 | + test "a ban needs exactly one scope" do | |
| 70 | + user = user_fixture() | |
| 71 | + | |
| 72 | + assert {:error, cs} = | |
| 73 | + %GitGud.Moderation.Ban{} | |
| 74 | + |> GitGud.Moderation.Ban.changeset(%{banned_user_id: user.id}) | |
| 75 | + |> GitGud.Repo.insert() | |
| 76 | + | |
| 77 | + refute cs.valid? | |
| 78 | + end | |
| 79 | + | |
| 80 | + test "anonymous visitors are never banned" do | |
| 81 | + {_owner, repo} = repository_fixture() | |
| 82 | + refute Moderation.banned?(repo, nil) | |
| 83 | + end | |
| 84 | + end | |
| 85 | + | |
| 86 | + describe "lifting and expiry" do | |
| 87 | + test "a lifted ban stops applying" do | |
| 88 | + {owner, repo} = repository_fixture() | |
| 89 | + nuisance = user_fixture() | |
| 90 | + {:ok, ban} = Moderation.ban(repo, nuisance) | |
| 91 | + | |
| 92 | + {:ok, _} = Moderation.lift(ban, owner) | |
| 93 | + | |
| 94 | + refute Moderation.banned?(repo, nuisance) | |
| 95 | + end | |
| 96 | + | |
| 97 | + test "a lifted ban is kept as a record" do | |
| 98 | + {owner, repo} = repository_fixture() | |
| 99 | + nuisance = user_fixture() | |
| 100 | + {:ok, ban} = Moderation.ban(repo, nuisance, reason: "spam") | |
| 101 | + {:ok, _} = Moderation.lift(ban, owner) | |
| 102 | + | |
| 103 | + assert [kept] = Moderation.list_bans(repo) | |
| 104 | + assert kept.reason == "spam" | |
| 105 | + assert kept.lifted_at | |
| 106 | + assert kept.lifted_by_id == owner.id | |
| 107 | + end | |
| 108 | + | |
| 109 | + test "an expired ban stops applying without any sweeper" do | |
| 110 | + {_owner, repo} = repository_fixture() | |
| 111 | + nuisance = user_fixture() | |
| 112 | + | |
| 113 | + # Insert directly: the changeset rightly refuses a past expiry. | |
| 114 | + GitGud.Repo.insert!(%GitGud.Moderation.Ban{ | |
| 115 | + repository_id: repo.id, | |
| 116 | + banned_user_id: nuisance.id, | |
| 117 | + expires_at: DateTime.utc_now(:second) |> DateTime.add(-60, :second) | |
| 118 | + }) | |
| 119 | + | |
| 120 | + refute Moderation.banned?(repo, nuisance) | |
| 121 | + end | |
| 122 | + | |
| 123 | + test "a future expiry is still in force" do | |
| 124 | + {_owner, repo} = repository_fixture() | |
| 125 | + nuisance = user_fixture() | |
| 126 | + later = DateTime.utc_now(:second) |> DateTime.add(3600, :second) | |
| 127 | + | |
| 128 | + {:ok, _ban} = Moderation.ban(repo, nuisance, expires_at: later) | |
| 129 | + | |
| 130 | + assert Moderation.banned?(repo, nuisance) | |
| 131 | + end | |
| 132 | + | |
| 133 | + test "an expiry in the past is refused up front" do | |
| 134 | + {_owner, repo} = repository_fixture() | |
| 135 | + nuisance = user_fixture() | |
| 136 | + past = DateTime.utc_now(:second) |> DateTime.add(-60, :second) | |
| 137 | + | |
| 138 | + assert {:error, cs} = Moderation.ban(repo, nuisance, expires_at: past) | |
| 139 | + assert "must be in the future" in errors_on(cs).expires_at | |
| 140 | + end | |
| 141 | + | |
| 142 | + test "banning the same person twice in a scope is refused" do | |
| 143 | + {_owner, repo} = repository_fixture() | |
| 144 | + nuisance = user_fixture() | |
| 145 | + | |
| 146 | + {:ok, _} = Moderation.ban(repo, nuisance) | |
| 147 | + assert {:error, _cs} = Moderation.ban(repo, nuisance) | |
| 148 | + end | |
| 149 | + | |
| 150 | + test "they can be re-banned after a lift" do | |
| 151 | + {owner, repo} = repository_fixture() | |
| 152 | + nuisance = user_fixture() | |
| 153 | + {:ok, ban} = Moderation.ban(repo, nuisance) | |
| 154 | + {:ok, _} = Moderation.lift(ban, owner) | |
| 155 | + | |
| 156 | + assert {:ok, _} = Moderation.ban(repo, nuisance) | |
| 157 | + end | |
| 158 | + | |
| 159 | + test "an org ban outranks a repo one when reporting why" do | |
| 160 | + {_admin, org, repo} = org_repo("mod-precedence") | |
| 161 | + nuisance = user_fixture() | |
| 162 | + | |
| 163 | + {:ok, _repo_ban} = Moderation.ban(repo, nuisance, reason: "repo level") | |
| 164 | + {:ok, _org_ban} = Moderation.ban(org, nuisance, reason: "org level") | |
| 165 | + | |
| 166 | + assert Moderation.active_ban(repo, nuisance).reason == "org level" | |
| 167 | + end | |
| 168 | + end | |
| 169 | + | |
| 170 | + describe "enforcement" do | |
| 171 | + setup do | |
| 172 | + {owner, repo} = repository_fixture(%{visibility: "public"}) | |
| 173 | + nuisance = user_fixture() | |
| 174 | + {:ok, _ban} = Moderation.ban(repo, nuisance, reason: "spam") | |
| 175 | + {:ok, owner: owner, repo: repo, nuisance: nuisance} | |
| 176 | + end | |
| 177 | + | |
| 178 | + test "can't open an issue", %{repo: repo, nuisance: nuisance} do | |
| 179 | + assert {:error, :banned} = Issues.create_issue(repo, nuisance, %{"title" => "hello"}) | |
| 180 | + end | |
| 181 | + | |
| 182 | + test "can't comment on an issue", %{owner: owner, repo: repo, nuisance: nuisance} do | |
| 183 | + issue = issue_fixture(repo, owner) | |
| 184 | + assert {:error, :banned} = Issues.add_comment(issue, nuisance, %{"body" => "spam"}) | |
| 185 | + end | |
| 186 | + | |
| 187 | + test "can't comment on a PR", %{owner: owner, repo: repo, nuisance: nuisance} do | |
| 188 | + pr = pr_with_branches(repo, owner) | |
| 189 | + assert {:error, :banned} = PullRequests.add_comment(pr, nuisance, %{"body" => "spam"}) | |
| 190 | + end | |
| 191 | + | |
| 192 | + test "can't review a PR", %{owner: owner, repo: repo, nuisance: nuisance} do | |
| 193 | + pr = pr_with_branches(repo, owner) | |
| 194 | + assert {:error, :banned} = PullRequests.add_review(pr, nuisance, %{"state" => "approved"}) | |
| 195 | + end | |
| 196 | + | |
| 197 | + test "can't be asked to review", %{owner: owner, repo: repo, nuisance: nuisance} do | |
| 198 | + pr = pr_with_branches(repo, owner) | |
| 199 | + assert {:error, :banned} = PullRequests.request_review(pr, nuisance, owner) | |
| 200 | + end | |
| 201 | + | |
| 202 | + test "can't open a PR", %{repo: repo, nuisance: nuisance} do | |
| 203 | + # Give them something to propose. | |
| 204 | + seed_main_branch(repo, nuisance) | |
| 205 | + | |
| 206 | + assert {:error, :banned} = | |
| 207 | + PullRequests.create_pull_request(repo, nuisance, %{ | |
| 208 | + "title" => "nope", | |
| 209 | + "source_ref" => "main", | |
| 210 | + "target_ref" => "main" | |
| 211 | + }) | |
| 212 | + end | |
| 213 | + | |
| 214 | + test "everyone else is unaffected", %{owner: owner, repo: repo} do | |
| 215 | + assert {:ok, _} = Issues.create_issue(repo, owner, %{"title" => "fine"}) | |
| 216 | + end | |
| 217 | + | |
| 218 | + test "and they can act again once lifted", %{owner: owner, repo: repo, nuisance: nuisance} do | |
| 219 | + [ban] = Moderation.list_bans(repo) | |
| 220 | + {:ok, _} = Moderation.lift(ban, owner) | |
| 221 | + | |
| 222 | + assert {:ok, _} = Issues.create_issue(repo, nuisance, %{"title" => "back"}) | |
| 223 | + end | |
| 224 | + end | |
| 225 | + | |
| 226 | + test "banned_user_ids covers both scopes" do | |
| 227 | + {_admin, org, repo} = org_repo("mod-ids") | |
| 228 | + org_banned = user_fixture() | |
| 229 | + repo_banned = user_fixture() | |
| 230 | + | |
| 231 | + {:ok, _} = Moderation.ban(org, org_banned) | |
| 232 | + {:ok, _} = Moderation.ban(repo, repo_banned) | |
| 233 | + | |
| 234 | + ids = Moderation.banned_user_ids(repo) | |
| 235 | + assert org_banned.id in ids | |
| 236 | + assert repo_banned.id in ids | |
| 237 | + end | |
| 238 | +end |
Parents: d2fd801