tenseleyflow/shithub / 38697c4

Browse files

admin: split repoForceArchive into Archive/Unarchive (SR2 H8 + M2/M3/M6)

H8: pre-fix repoForceArchive flipped is_archived as a toggle —
clicking on an already-archived repo silently un-archived it with
audit row ActionAdminRepoForceArchived (the name lied on the
unarchive path). Now two routes:
- POST /admin/repos/{id}/archive → repoForceArchive (idempotent)
- POST /admin/repos/{id}/unarchive → repoForceUnarchive
Each emits its own audit action (ActionAdminRepoForceArchived /
ActionAdminRepoForceUnarchived). repo_view.html branches on
IsArchived to render the correct button.

M2: inline raw SQL in admin handlers replaced with sqlc-generated
queries:
- admin/repos.repoForceArchive → reposdb.ArchiveRepo
- admin/repos.repoForceUnarchive → reposdb.UnarchiveRepo (existed)
- admin/repos.repoForceDelete → reposdb.AdminForceDeleteRepo (new)
- admin/users.userUnsuspend → usersdb.UnsuspendUser (new)
- orgs/teams.canSeeTeam → orgsdb.IsTeamMember (new)
- orgs/teams.filterSecretTeams → orgsdb.IsTeamMember (same query)

M3: drop the wasted ListTeamMembers call in canSeeTeam — the
result was thrown into _ before the actual EXISTS check ran.

M6 (partial): admin/repos.go switches http.Error → renderer for
all error paths in the touched handlers.
Authored by espadonne
SHA
38697c4703c232d72f0d7f4d08f3108c55e1024f
Parents
e5aab86
Tree
0b8a472

14 changed files

StatusFile+-
M internal/orgs/queries/teams.sql 6 0
M internal/orgs/sqlc/querier.go 4 0
M internal/orgs/sqlc/teams.sql.go 19 0
M internal/repos/queries/repos.sql 9 0
M internal/repos/sqlc/querier.go 5 0
M internal/repos/sqlc/repos.sql.go 13 0
M internal/users/queries/users.sql 9 0
M internal/users/sqlc/querier.go 4 0
M internal/users/sqlc/users.sql.go 15 0
M internal/web/handlers/admin/admin.go 4 0
M internal/web/handlers/admin/repos.go 26 12
M internal/web/handlers/admin/users.go 5 7
M internal/web/handlers/orgs/teams.go 9 14
M internal/web/templates/admin/repo_view.html 8 1
internal/orgs/queries/teams.sqlmodified
@@ -107,3 +107,9 @@ ORDER BY t.slug ASC;
107107
 SELECT team_id, repo_id, role
108108
 FROM team_repo_access
109109
 WHERE repo_id = $1 AND team_id = ANY($2::bigint[]);
110
+
111
+-- name: IsTeamMember :one
112
+-- Replaces the inline EXISTS query in handlers/orgs/teams.go
113
+-- canSeeTeam + filterSecretTeams (SR2 M2). Used by the visibility
114
+-- gate for secret teams.
115
+SELECT EXISTS(SELECT 1 FROM team_members WHERE team_id = $1 AND user_id = $2);
internal/orgs/sqlc/querier.gomodified
@@ -64,6 +64,10 @@ type Querier interface {
6464
 	// Final row removal after the cascade finished. The principals
6565
 	// trigger drops the matching principals row in the same tx.
6666
 	HardDeleteOrgRow(ctx context.Context, db DBTX, id int64) error
67
+	// Replaces the inline EXISTS query in handlers/orgs/teams.go
68
+	// canSeeTeam + filterSecretTeams (SR2 M2). Used by the visibility
69
+	// gate for secret teams.
70
+	IsTeamMember(ctx context.Context, db DBTX, arg IsTeamMemberParams) (bool, error)
6771
 	ListChildTeams(ctx context.Context, db DBTX, parentTeamID pgtype.Int8) ([]Team, error)
6872
 	// Sweep input for the lifecycle worker: every soft-deleted org whose
6973
 	// 14-day grace window has elapsed. The interval is intentionally a
internal/orgs/sqlc/teams.sql.gomodified
@@ -190,6 +190,25 @@ func (q *Queries) GrantTeamRepoAccess(ctx context.Context, db DBTX, arg GrantTea
190190
 	return err
191191
 }
192192
193
+const isTeamMember = `-- name: IsTeamMember :one
194
+SELECT EXISTS(SELECT 1 FROM team_members WHERE team_id = $1 AND user_id = $2)
195
+`
196
+
197
+type IsTeamMemberParams struct {
198
+	TeamID int64
199
+	UserID int64
200
+}
201
+
202
+// Replaces the inline EXISTS query in handlers/orgs/teams.go
203
+// canSeeTeam + filterSecretTeams (SR2 M2). Used by the visibility
204
+// gate for secret teams.
205
+func (q *Queries) IsTeamMember(ctx context.Context, db DBTX, arg IsTeamMemberParams) (bool, error) {
206
+	row := db.QueryRow(ctx, isTeamMember, arg.TeamID, arg.UserID)
207
+	var exists bool
208
+	err := row.Scan(&exists)
209
+	return exists, err
210
+}
211
+
193212
 const listChildTeams = `-- name: ListChildTeams :many
194213
 SELECT id, org_id, slug, display_name, description, parent_team_id, privacy, created_by_user_id, created_at, updated_at FROM teams WHERE parent_team_id = $1 ORDER BY slug ASC
195214
 `
internal/repos/queries/repos.sqlmodified
@@ -270,3 +270,12 @@ FROM repos r
270270
 JOIN users u ON u.id = r.owner_user_id
271271
 WHERE r.fork_of_repo_id = $1
272272
   AND r.deleted_at IS NULL;
273
+
274
+
275
+-- name: AdminForceDeleteRepo :exec
276
+-- Bypasses the soft-delete grace window (admin only — S34): set
277
+-- deleted_at to a year ago so the next lifecycle sweep hard-deletes
278
+-- without waiting. Replaces the inline UPDATE in admin/repos.go
279
+-- (SR2 M2).
280
+UPDATE repos SET deleted_at = now() - interval '1 year' WHERE id = $1;
281
+
internal/repos/sqlc/querier.gomodified
@@ -12,6 +12,11 @@ import (
1212
1313
 type Querier interface {
1414
 	AcceptTransferRequest(ctx context.Context, db DBTX, id int64) error
15
+	// Bypasses the soft-delete grace window (admin only — S34): set
16
+	// deleted_at to a year ago so the next lifecycle sweep hard-deletes
17
+	// without waiting. Replaces the inline UPDATE in admin/repos.go
18
+	// (SR2 M2).
19
+	AdminForceDeleteRepo(ctx context.Context, db DBTX, id int64) error
1520
 	ArchiveRepo(ctx context.Context, db DBTX, id int64) error
1621
 	CancelTransferRequest(ctx context.Context, db DBTX, id int64) error
1722
 	CountForksOfRepo(ctx context.Context, db DBTX, forkOfRepoID pgtype.Int8) (int64, error)
internal/repos/sqlc/repos.sql.gomodified
@@ -11,6 +11,19 @@ import (
1111
 	"github.com/jackc/pgx/v5/pgtype"
1212
 )
1313
14
+const adminForceDeleteRepo = `-- name: AdminForceDeleteRepo :exec
15
+UPDATE repos SET deleted_at = now() - interval '1 year' WHERE id = $1
16
+`
17
+
18
+// Bypasses the soft-delete grace window (admin only — S34): set
19
+// deleted_at to a year ago so the next lifecycle sweep hard-deletes
20
+// without waiting. Replaces the inline UPDATE in admin/repos.go
21
+// (SR2 M2).
22
+func (q *Queries) AdminForceDeleteRepo(ctx context.Context, db DBTX, id int64) error {
23
+	_, err := db.Exec(ctx, adminForceDeleteRepo, id)
24
+	return err
25
+}
26
+
1427
 const countForksOfRepo = `-- name: CountForksOfRepo :one
1528
 SELECT count(*) FROM repos
1629
 WHERE fork_of_repo_id = $1 AND deleted_at IS NULL
internal/users/queries/users.sqlmodified
@@ -47,6 +47,15 @@ SET suspended_at = now(),
4747
     suspended_reason = $2
4848
 WHERE id = $1;
4949
50
+-- name: UnsuspendUser :exec
51
+-- Clears the suspended state. Mirrors SuspendUser; used by the
52
+-- /admin/users/{id}/unsuspend handler. Replaces an inline UPDATE
53
+-- in admin/users.go (SR2 M2).
54
+UPDATE users
55
+SET suspended_at     = NULL,
56
+    suspended_reason = NULL
57
+WHERE id = $1;
58
+
5059
 -- name: SoftDeleteUser :exec
5160
 UPDATE users
5261
 SET deleted_at = now()
internal/users/sqlc/querier.gomodified
@@ -128,6 +128,10 @@ type Querier interface {
128128
 	TouchSSHKeyLastUsed(ctx context.Context, db DBTX, arg TouchSSHKeyLastUsedParams) error
129129
 	TouchUserLastLogin(ctx context.Context, db DBTX, id int64) error
130130
 	TouchUserTokenLastUsed(ctx context.Context, db DBTX, arg TouchUserTokenLastUsedParams) error
131
+	// Clears the suspended state. Mirrors SuspendUser; used by the
132
+	// /admin/users/{id}/unsuspend handler. Replaces an inline UPDATE
133
+	// in admin/users.go (SR2 M2).
134
+	UnsuspendUser(ctx context.Context, db DBTX, id int64) error
131135
 	UpdateUserAvatarKey(ctx context.Context, db DBTX, arg UpdateUserAvatarKeyParams) error
132136
 	UpdateUserPassword(ctx context.Context, db DBTX, arg UpdateUserPasswordParams) error
133137
 	UpdateUserProfile(ctx context.Context, db DBTX, arg UpdateUserProfileParams) error
internal/users/sqlc/users.sql.gomodified
@@ -349,6 +349,21 @@ func (q *Queries) TouchUserLastLogin(ctx context.Context, db DBTX, id int64) err
349349
 	return err
350350
 }
351351
352
+const unsuspendUser = `-- name: UnsuspendUser :exec
353
+UPDATE users
354
+SET suspended_at     = NULL,
355
+    suspended_reason = NULL
356
+WHERE id = $1
357
+`
358
+
359
+// Clears the suspended state. Mirrors SuspendUser; used by the
360
+// /admin/users/{id}/unsuspend handler. Replaces an inline UPDATE
361
+// in admin/users.go (SR2 M2).
362
+func (q *Queries) UnsuspendUser(ctx context.Context, db DBTX, id int64) error {
363
+	_, err := db.Exec(ctx, unsuspendUser, id)
364
+	return err
365
+}
366
+
352367
 const updateUserAvatarKey = `-- name: UpdateUserAvatarKey :exec
353368
 UPDATE users
354369
 SET avatar_object_key = $2
internal/web/handlers/admin/admin.gomodified
@@ -75,7 +75,11 @@ func (h *Handlers) Mount(r chi.Router) {
7575
 	// Repos
7676
 	r.Get("/admin/repos", h.reposList)
7777
 	r.Get("/admin/repos/{id}", h.repoView)
78
+	// Archive / Unarchive split into distinct routes (SR2 H8):
79
+	// pre-split, /archive was a toggle, so re-clicking on an already-
80
+	// archived repo silently un-archived with a misleading audit row.
7881
 	r.Post("/admin/repos/{id}/archive", h.repoForceArchive)
82
+	r.Post("/admin/repos/{id}/unarchive", h.repoForceUnarchive)
7983
 	r.Post("/admin/repos/{id}/delete", h.repoForceDelete)
8084
8185
 	// Jobs
internal/web/handlers/admin/repos.gomodified
@@ -74,23 +74,39 @@ func (h *Handlers) repoView(w http.ResponseWriter, r *http.Request) {
7474
 	})
7575
 }
7676
77
-// repoForceArchive flips is_archived without owner consent. Used for
78
-// emergency takedown — the normal Archive flow is owner-only.
77
+// repoForceArchive forces is_archived=true without owner consent.
78
+// Used for emergency takedown — the normal Archive flow is owner-only.
79
+// Idempotent: a no-op when the repo is already archived (still audited).
7980
 func (h *Handlers) repoForceArchive(w http.ResponseWriter, r *http.Request) {
8081
 	row, ok := h.loadRepo(w, r)
8182
 	if !ok {
8283
 		return
8384
 	}
84
-	if _, err := h.d.Pool.Exec(r.Context(),
85
-		`UPDATE repos SET is_archived = NOT is_archived, archived_at = CASE WHEN is_archived THEN NULL ELSE now() END WHERE id = $1`,
86
-		row.ID); err != nil {
87
-		http.Error(w, "force-archive failed", http.StatusInternalServerError)
85
+	if err := reposdb.New().ArchiveRepo(r.Context(), h.d.Pool, row.ID); err != nil {
86
+		h.d.Render.HTTPError(w, r, http.StatusInternalServerError, "force-archive failed")
8887
 		return
8988
 	}
9089
 	h.recordAdminAction(r, audit.ActionAdminRepoForceArchived, audit.TargetRepo, row.ID, nil)
9190
 	http.Redirect(w, r, "/admin/repos/"+strconv.FormatInt(row.ID, 10)+"?notice=saved", http.StatusSeeOther)
9291
 }
9392
93
+// repoForceUnarchive forces is_archived=false. Separate route from
94
+// repoForceArchive (split per SR2 H8 — pre-fix the single route was
95
+// a toggle, so clicking on an already-archived repo silently
96
+// un-archived it with a misleading audit row).
97
+func (h *Handlers) repoForceUnarchive(w http.ResponseWriter, r *http.Request) {
98
+	row, ok := h.loadRepo(w, r)
99
+	if !ok {
100
+		return
101
+	}
102
+	if err := reposdb.New().UnarchiveRepo(r.Context(), h.d.Pool, row.ID); err != nil {
103
+		h.d.Render.HTTPError(w, r, http.StatusInternalServerError, "force-unarchive failed")
104
+		return
105
+	}
106
+	h.recordAdminAction(r, audit.ActionAdminRepoForceUnarchived, audit.TargetRepo, row.ID, nil)
107
+	http.Redirect(w, r, "/admin/repos/"+strconv.FormatInt(row.ID, 10)+"?notice=saved", http.StatusSeeOther)
108
+}
109
+
94110
 // repoForceDelete bypasses the soft-delete grace by setting deleted_at
95111
 // to a time well past the grace window, then hard-delete sweeps it on
96112
 // the next lifecycle:sweep tick.
@@ -100,18 +116,16 @@ func (h *Handlers) repoForceDelete(w http.ResponseWriter, r *http.Request) {
100116
 		return
101117
 	}
102118
 	if err := r.ParseForm(); err != nil {
103
-		http.Error(w, "form parse", http.StatusBadRequest)
119
+		h.d.Render.HTTPError(w, r, http.StatusBadRequest, "form parse")
104120
 		return
105121
 	}
106122
 	confirm := strings.TrimSpace(r.PostFormValue("confirm"))
107123
 	if confirm != row.Name {
108
-		http.Error(w, "confirmation text didn't match the repo name", http.StatusBadRequest)
124
+		h.d.Render.HTTPError(w, r, http.StatusBadRequest, "confirmation text didn't match the repo name")
109125
 		return
110126
 	}
111
-	if _, err := h.d.Pool.Exec(r.Context(),
112
-		`UPDATE repos SET deleted_at = now() - interval '1 year' WHERE id = $1`,
113
-		row.ID); err != nil {
114
-		http.Error(w, "force-delete failed", http.StatusInternalServerError)
127
+	if err := reposdb.New().AdminForceDeleteRepo(r.Context(), h.d.Pool, row.ID); err != nil {
128
+		h.d.Render.HTTPError(w, r, http.StatusInternalServerError, "force-delete failed")
115129
 		return
116130
 	}
117131
 	h.recordAdminAction(r, audit.ActionAdminRepoForceDeleted, audit.TargetRepo, row.ID,
internal/web/handlers/admin/users.gomodified
@@ -113,13 +113,11 @@ func (h *Handlers) userUnsuspend(w http.ResponseWriter, r *http.Request) {
113113
 		http.Error(w, "unsuspend failed", http.StatusInternalServerError)
114114
 		return
115115
 	}
116
-	// SuspendUser zeros the reason but leaves suspended_at; clear it
117
-	// directly so the user reads as active again. (The unsuspend path
118
-	// is admin-only so we lean on a one-off SQL exec rather than
119
-	// extending the users sqlc surface.)
120
-	if _, err := h.d.Pool.Exec(r.Context(),
121
-		`UPDATE users SET suspended_at = NULL, suspended_reason = NULL WHERE id = $1`,
122
-		user.ID); err != nil {
116
+	// SR2 M2: was inline SQL with the comment "lean on a one-off SQL
117
+	// exec rather than extending the users sqlc surface." That comment
118
+	// stopped being true the moment we needed to test it; usersdb has
119
+	// the UnsuspendUser query now.
120
+	if err := h.uq.UnsuspendUser(r.Context(), h.d.Pool, user.ID); err != nil {
123121
 		h.d.Logger.WarnContext(r.Context(), "admin: unsuspend clear", "error", err)
124122
 	}
125123
 	h.recordAdminAction(r, audit.ActionAdminUserUnsuspended, audit.TargetUser, user.ID, nil)
internal/web/handlers/orgs/teams.gomodified
@@ -259,17 +259,14 @@ func (h *Handlers) canSeeTeam(r *http.Request, team orgsdb.Team, viewer middlewa
259259
 	if owner, _ := orgs.IsOwner(r.Context(), h.deps(), team.OrgID, viewer.ID); owner {
260260
 		return true
261261
 	}
262
-	// Team member?
263
-	_, err := orgsdb.New().ListTeamMembers(r.Context(), h.d.Pool, team.ID)
262
+	// Team member? (SR2 M2 + M3: was an inline EXISTS preceded by a
263
+	// wasted ListTeamMembers call whose result was dropped with `_`.)
264
+	member, err := orgsdb.New().IsTeamMember(r.Context(), h.d.Pool, orgsdb.IsTeamMemberParams{
265
+		TeamID: team.ID, UserID: viewer.ID,
266
+	})
264267
 	if err != nil {
265268
 		return false
266269
 	}
267
-	var member bool
268
-	_ = h.d.Pool.QueryRow(
269
-		r.Context(),
270
-		`SELECT EXISTS(SELECT 1 FROM team_members WHERE team_id = $1 AND user_id = $2)`,
271
-		team.ID, viewer.ID,
272
-	).Scan(&member)
273270
 	return member
274271
 }
275272
@@ -292,12 +289,10 @@ func (h *Handlers) filterSecretTeams(r *http.Request, all []orgsdb.Team, orgID i
292289
 		if viewer.IsAnonymous() {
293290
 			continue
294291
 		}
295
-		var member bool
296
-		err := h.d.Pool.QueryRow(
297
-			r.Context(),
298
-			`SELECT EXISTS(SELECT 1 FROM team_members WHERE team_id = $1 AND user_id = $2)`,
299
-			t.ID, viewer.ID,
300
-		).Scan(&member)
292
+		// SR2 M2: was an inline EXISTS query.
293
+		member, err := orgsdb.New().IsTeamMember(r.Context(), h.d.Pool, orgsdb.IsTeamMemberParams{
294
+			TeamID: t.ID, UserID: viewer.ID,
295
+		})
301296
 		if err == nil && member {
302297
 			out = append(out, t)
303298
 		}
internal/web/templates/admin/repo_view.htmlmodified
@@ -21,10 +21,17 @@
2121
     <section class="shithub-settings-section">
2222
       <h2>Force actions</h2>
2323
       <p class="shithub-hint">These bypass owner consent. Use only for emergency takedown / abuse response.</p>
24
+      {{ if .Repo.IsArchived }}
25
+      <form method="POST" action="/admin/repos/{{ .Repo.ID }}/unarchive" style="display:inline">
26
+        <input type="hidden" name="csrf_token" value="{{ .CSRFToken }}">
27
+        <button type="submit" class="shithub-button">Force-unarchive</button>
28
+      </form>
29
+      {{ else }}
2430
       <form method="POST" action="/admin/repos/{{ .Repo.ID }}/archive" style="display:inline">
2531
         <input type="hidden" name="csrf_token" value="{{ .CSRFToken }}">
26
-        <button type="submit" class="shithub-button">{{ if .Repo.IsArchived }}Force-unarchive{{ else }}Force-archive{{ end }}</button>
32
+        <button type="submit" class="shithub-button">Force-archive</button>
2733
       </form>
34
+      {{ end }}
2835
2936
       <form method="POST" action="/admin/repos/{{ .Repo.ID }}/delete">
3037
         <input type="hidden" name="csrf_token" value="{{ .CSRFToken }}">