tenseleyflow/shithub / cad7ad5

Browse files

audit: webhook actions get their own enums + missing trails (SR2 H1+M4)

Pre-fix: webhook create/update overloaded ActionRepoCreated with
a meta.action discriminator. Webhook delete/toggle/ping/redeliver
were not audited at all — admin had no forensic record of who
disabled, deleted, or replayed a hook.

Post-fix:
- New audit actions: ActionWebhook{Created,Updated,Deleted,
ActiveSet,ActiveUnset,Pinged,Redelivered}.
- All 7 webhook handlers now audit, all attributed via
viewer.AuditActor so impersonation trails carry.
- Bonus: ActionAdminRepoForceUnarchived added in preparation for
SR2 H8 (split repoForceArchive into archive/unarchive).
Authored by espadonne
SHA
cad7ad5e52c51035dd871b333d96ce7f2ab2f587
Parents
03475a3
Tree
be99bf4

2 changed files

StatusFile+-
M internal/auth/audit/audit.go 25 13
M internal/web/handlers/repo/webhooks.go 31 7
internal/auth/audit/audit.gomodified
@@ -72,22 +72,34 @@ const (
7272
 	ActionRepoForked            Action = "repo_forked"
7373
 	ActionRepoForkSynced        Action = "repo_fork_synced"
7474
75
+	// S33 / SR2 H1 — webhook lifecycle. Pre-SR2 webhook create/update
76
+	// overloaded ActionRepoCreated; delete/toggle/ping/redeliver were
77
+	// not audited at all. Each event now has its own enum.
78
+	ActionWebhookCreated     Action = "webhook_created"
79
+	ActionWebhookUpdated     Action = "webhook_updated"
80
+	ActionWebhookDeleted     Action = "webhook_deleted"
81
+	ActionWebhookActiveSet   Action = "webhook_active_set"
82
+	ActionWebhookActiveUnset Action = "webhook_active_unset"
83
+	ActionWebhookPinged      Action = "webhook_pinged"
84
+	ActionWebhookRedelivered Action = "webhook_redelivered"
85
+
7586
 	// S34 — site admin actions. Always recorded with the real admin's
7687
 	// id in actor_id; impersonation flows additionally carry the
7788
 	// impersonated user's id in meta.impersonated_user_id.
78
-	ActionAdminSiteAdminGranted   Action = "admin_site_admin_granted"
79
-	ActionAdminSiteAdminRevoked   Action = "admin_site_admin_revoked"
80
-	ActionAdminUserSuspended      Action = "admin_user_suspended"
81
-	ActionAdminUserUnsuspended    Action = "admin_user_unsuspended"
82
-	ActionAdminUserForceDeleted   Action = "admin_user_force_deleted"
83
-	ActionAdminUserPasswordReset  Action = "admin_user_password_reset"
84
-	ActionAdminRepoForceArchived  Action = "admin_repo_force_archived"
85
-	ActionAdminRepoForceDeleted   Action = "admin_repo_force_deleted"
86
-	ActionAdminJobRetried         Action = "admin_job_retried"
87
-	ActionAdminJobDiscarded       Action = "admin_job_discarded"
88
-	ActionAdminImpersonateStarted Action = "admin_impersonate_started"
89
-	ActionAdminImpersonateStopped Action = "admin_impersonate_stopped"
90
-	ActionAdminImpersonateWriteOn Action = "admin_impersonate_write_on"
89
+	ActionAdminSiteAdminGranted    Action = "admin_site_admin_granted"
90
+	ActionAdminSiteAdminRevoked    Action = "admin_site_admin_revoked"
91
+	ActionAdminUserSuspended       Action = "admin_user_suspended"
92
+	ActionAdminUserUnsuspended     Action = "admin_user_unsuspended"
93
+	ActionAdminUserForceDeleted    Action = "admin_user_force_deleted"
94
+	ActionAdminUserPasswordReset   Action = "admin_user_password_reset"
95
+	ActionAdminRepoForceArchived   Action = "admin_repo_force_archived"
96
+	ActionAdminRepoForceUnarchived Action = "admin_repo_force_unarchived"
97
+	ActionAdminRepoForceDeleted    Action = "admin_repo_force_deleted"
98
+	ActionAdminJobRetried          Action = "admin_job_retried"
99
+	ActionAdminJobDiscarded        Action = "admin_job_discarded"
100
+	ActionAdminImpersonateStarted  Action = "admin_impersonate_started"
101
+	ActionAdminImpersonateStopped  Action = "admin_impersonate_stopped"
102
+	ActionAdminImpersonateWriteOn  Action = "admin_impersonate_write_on"
91103
 )
92104
93105
 // Target is a typed target-type constant.
internal/web/handlers/repo/webhooks.gomodified
@@ -118,9 +118,9 @@ func (h *Handlers) webhookCreate(w http.ResponseWriter, r *http.Request) {
118118
 		}, friendlyWebhookError(err), "")
119119
 		return
120120
 	}
121
-	auditActor, auditMeta := viewer.AuditActor(map[string]any{"action": "webhook_created", "webhook_id": created.ID, "url": params.URL})
121
+	auditActor, auditMeta := viewer.AuditActor(map[string]any{"webhook_id": created.ID, "url": params.URL})
122122
 	_ = h.d.Audit.Record(r.Context(), h.d.Pool, auditActor,
123
-		audit.ActionRepoCreated, audit.TargetRepo, row.ID,
123
+		audit.ActionWebhookCreated, audit.TargetRepo, row.ID,
124124
 		auditMeta)
125125
126126
 	http.Redirect(w, r, "/"+owner.Username+"/"+row.Name+"/settings/webhooks?notice=saved", http.StatusSeeOther)
@@ -180,9 +180,9 @@ func (h *Handlers) webhookUpdate(w http.ResponseWriter, r *http.Request) {
180180
 		return
181181
 	}
182182
 	viewer := middleware.CurrentUserFromContext(r.Context())
183
-	auditActor, auditMeta := viewer.AuditActor(map[string]any{"action": "webhook_updated", "webhook_id": hook.ID})
183
+	auditActor, auditMeta := viewer.AuditActor(map[string]any{"webhook_id": hook.ID})
184184
 	_ = h.d.Audit.Record(r.Context(), h.d.Pool, auditActor,
185
-		audit.ActionRepoCreated, audit.TargetRepo, row.ID,
185
+		audit.ActionWebhookUpdated, audit.TargetRepo, row.ID,
186186
 		auditMeta)
187187
 	http.Redirect(w, r, "/"+owner.Username+"/"+row.Name+"/settings/webhooks/"+strconv.FormatInt(hook.ID, 10)+"?notice=saved", http.StatusSeeOther)
188188
 }
@@ -204,9 +204,9 @@ func (h *Handlers) webhookDelete(w http.ResponseWriter, r *http.Request) {
204204
 		return
205205
 	}
206206
 	viewer := middleware.CurrentUserFromContext(r.Context())
207
-	auditActor, auditMeta := viewer.AuditActor(map[string]any{"action": "webhook_deleted", "webhook_id": hook.ID})
207
+	auditActor, auditMeta := viewer.AuditActor(map[string]any{"webhook_id": hook.ID, "url": hook.Url})
208208
 	_ = h.d.Audit.Record(r.Context(), h.d.Pool, auditActor,
209
-		audit.ActionRepoCreated, audit.TargetRepo, row.ID,
209
+		audit.ActionWebhookDeleted, audit.TargetRepo, row.ID,
210210
 		auditMeta)
211211
 	http.Redirect(w, r, "/"+owner.Username+"/"+row.Name+"/settings/webhooks?notice=saved", http.StatusSeeOther)
212212
 }
@@ -222,12 +222,22 @@ func (h *Handlers) webhookToggle(w http.ResponseWriter, r *http.Request) {
222222
 	if !ok {
223223
 		return
224224
 	}
225
+	newActive := !hook.Active
225226
 	if err := webhook.SetActive(r.Context(), webhook.ManageDeps{
226227
 		Pool: h.d.Pool, SecretBox: h.d.SecretBox,
227
-	}, hook.ID, !hook.Active); err != nil {
228
+	}, hook.ID, newActive); err != nil {
228229
 		http.Error(w, "toggle failed", http.StatusInternalServerError)
229230
 		return
230231
 	}
232
+	viewer := middleware.CurrentUserFromContext(r.Context())
233
+	action := audit.ActionWebhookActiveSet
234
+	if !newActive {
235
+		action = audit.ActionWebhookActiveUnset
236
+	}
237
+	auditActor, auditMeta := viewer.AuditActor(map[string]any{"webhook_id": hook.ID})
238
+	_ = h.d.Audit.Record(r.Context(), h.d.Pool, auditActor,
239
+		action, audit.TargetRepo, row.ID,
240
+		auditMeta)
231241
 	http.Redirect(w, r, "/"+owner.Username+"/"+row.Name+"/settings/webhooks/"+strconv.FormatInt(hook.ID, 10)+"?notice=saved", http.StatusSeeOther)
232242
 }
233243
@@ -246,6 +256,11 @@ func (h *Handlers) webhookPing(w http.ResponseWriter, r *http.Request) {
246256
 	}, hook.ID); err != nil {
247257
 		h.d.Logger.WarnContext(r.Context(), "webhook ping", "error", err)
248258
 	}
259
+	viewer := middleware.CurrentUserFromContext(r.Context())
260
+	auditActor, auditMeta := viewer.AuditActor(map[string]any{"webhook_id": hook.ID})
261
+	_ = h.d.Audit.Record(r.Context(), h.d.Pool, auditActor,
262
+		audit.ActionWebhookPinged, audit.TargetRepo, row.ID,
263
+		auditMeta)
249264
 	http.Redirect(w, r, "/"+owner.Username+"/"+row.Name+"/settings/webhooks/"+strconv.FormatInt(hook.ID, 10)+"?notice=saved", http.StatusSeeOther)
250265
 }
251266
@@ -310,6 +325,15 @@ func (h *Handlers) webhookRedeliver(w http.ResponseWriter, r *http.Request) {
310325
 		http.Error(w, "redeliver failed", http.StatusInternalServerError)
311326
 		return
312327
 	}
328
+	viewer := middleware.CurrentUserFromContext(r.Context())
329
+	auditActor, auditMeta := viewer.AuditActor(map[string]any{
330
+		"webhook_id":           hook.ID,
331
+		"original_delivery_id": originalID,
332
+		"new_delivery_id":      newID,
333
+	})
334
+	_ = h.d.Audit.Record(r.Context(), h.d.Pool, auditActor,
335
+		audit.ActionWebhookRedelivered, audit.TargetRepo, row.ID,
336
+		auditMeta)
313337
 	http.Redirect(w, r, "/"+owner.Username+"/"+row.Name+"/settings/webhooks/"+strconv.FormatInt(hook.ID, 10)+"/deliveries/"+strconv.FormatInt(newID, 10), http.StatusSeeOther)
314338
 }
315339