feat(audit): stamp actor on execution + RI-exchange state transitions (closes #1009)

Add transitioned_by/transitioned_at columns to purchase_executions,
ri_exchange_history, and account_registrations (migration 000066).
Thread an optional actor *string through all three TransitionStatus
methods so human-initiated transitions record the session user UUID
while system-initiated paths (reaper, scheduler, token-based approval)
pass nil. Guard non-UUID reviewer IDs (e.g. "admin-api-key") via
validUUIDPtrOrNil before using as a FK value. Mirrors the
CancelExecutionAtomic(actor) pattern from #804.

Author: cristim

internal/analytics/collector_test.go

@@ -241,7 +241,7 @@ func (m *mockConfigStore) ListPendingExecutionIDsForAccount(ctx context.Context,
 	return nil, nil
 }
 
-func (m *mockConfigStore) TransitionExecutionStatus(ctx context.Context, executionID string, fromStatuses []string, toStatus string) (*config.PurchaseExecution, error) {
+func (m *mockConfigStore) TransitionExecutionStatus(ctx context.Context, executionID string, fromStatuses []string, toStatus string, actor *string) (*config.PurchaseExecution, error) {
 	return nil, nil
 }
 
@@ -265,7 +265,7 @@ func (m *mockConfigStore) GetRIExchangeRecordByToken(ctx context.Context, token
 func (m *mockConfigStore) GetRIExchangeHistory(ctx context.Context, since time.Time, limit int) ([]config.RIExchangeRecord, error) {
 	return nil, nil
 }
-func (m *mockConfigStore) TransitionRIExchangeStatus(ctx context.Context, id string, fromStatus string, toStatus string) (*config.RIExchangeRecord, error) {
+func (m *mockConfigStore) TransitionRIExchangeStatus(ctx context.Context, id string, fromStatus string, toStatus string, actor *string) (*config.RIExchangeRecord, error) {
 	return nil, nil
 }
 func (m *mockConfigStore) CompleteRIExchange(ctx context.Context, id string, exchangeID string) error {
@@ -350,7 +350,7 @@ func (m *mockConfigStore) ListAccountRegistrations(_ context.Context, _ config.A
 func (m *mockConfigStore) UpdateAccountRegistration(_ context.Context, _ *config.AccountRegistration) error {
 	return nil
 }
-func (m *mockConfigStore) TransitionRegistrationStatus(_ context.Context, _ *config.AccountRegistration, _ string) error {
+func (m *mockConfigStore) TransitionRegistrationStatus(_ context.Context, _ *config.AccountRegistration, _ string, _ *string) error {
 	return nil
 }
 func (m *mockConfigStore) DeleteAccountRegistration(_ context.Context, _ string) error {

internal/api/coverage_extras_test.go

@@ -330,7 +330,7 @@ func TestHandler_rejectRIExchange_AlreadyProcessed(t *testing.T) {
 	mockStore.On("GetRIExchangeRecord", ctx, "11111111-1111-1111-1111-111111111111").Return(
 		&config.RIExchangeRecord{ID: "11111111-1111-1111-1111-111111111111", ApprovalToken: "tok"}, nil)
 	// Transition returns nil indicating already processed
-	mockStore.On("TransitionRIExchangeStatus", ctx, "11111111-1111-1111-1111-111111111111", "pending", "cancelled").
+	mockStore.On("TransitionRIExchangeStatus", ctx, "11111111-1111-1111-1111-111111111111", "pending", "cancelled", mock.Anything).
 		Return(nil, nil)
 
 	h := &Handler{config: mockStore}
@@ -364,11 +364,81 @@ func TestHandler_approveRIExchange_AlreadyProcessed(t *testing.T) {
 	mockStore := new(MockConfigStore)
 	mockStore.On("GetRIExchangeRecord", ctx, "11111111-1111-1111-1111-111111111111").Return(
 		&config.RIExchangeRecord{ID: "11111111-1111-1111-1111-111111111111", ApprovalToken: "tok"}, nil)
-	mockStore.On("TransitionRIExchangeStatus", ctx, "11111111-1111-1111-1111-111111111111", "pending", "processing").
+	mockStore.On("TransitionRIExchangeStatus", ctx, "11111111-1111-1111-1111-111111111111", "pending", "processing", mock.Anything).
 		Return(nil, nil)
 
 	h := &Handler{config: mockStore}
 	_, err := h.approveRIExchange(ctx, &events.LambdaFunctionURLRequest{}, "11111111-1111-1111-1111-111111111111", "tok")
 	assert.Error(t, err)
 	assert.Contains(t, err.Error(), "already processed")
 }
+
+// ---------------------------------------------------------------------------
+// validUUIDPtrOrNil — actor-stamp helper (issue #1009)
+// ---------------------------------------------------------------------------
+
+// TestValidUUIDPtrOrNil_ReturnsPointerForValidUUID asserts the happy-path:
+// a string that parses as a UUID is passed through unchanged.
+func TestValidUUIDPtrOrNil_ReturnsPointerForValidUUID(t *testing.T) {
+	uid := "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa"
+	result := validUUIDPtrOrNil(&uid)
+	require.NotNil(t, result, "valid UUID must return non-nil pointer")
+	assert.Equal(t, uid, *result)
+}
+
+// TestValidUUIDPtrOrNil_ReturnsNilForNonUUID asserts that a non-UUID string
+// (e.g. "admin-api-key") returns nil so it is never used as a FK actor.
+func TestValidUUIDPtrOrNil_ReturnsNilForNonUUID(t *testing.T) {
+	s := "admin-api-key"
+	assert.Nil(t, validUUIDPtrOrNil(&s), "non-UUID reviewer_by must map to nil actor")
+}
+
+// TestValidUUIDPtrOrNil_ReturnsNilForNilInput asserts that a nil *string
+// returns nil (no panic on nil dereference).
+func TestValidUUIDPtrOrNil_ReturnsNilForNilInput(t *testing.T) {
+	assert.Nil(t, validUUIDPtrOrNil(nil))
+}
+
+// TestHandler_TransitionRegistrationStatus_ActorStamped asserts that
+// TransitionRegistrationStatus is called with a non-nil actor when ReviewedBy
+// holds a valid UUID (the common human-reviewed path).
+func TestHandler_TransitionRegistrationStatus_ActorStamped(t *testing.T) {
+	ctx := context.Background()
+	mockStore := new(MockConfigStore)
+	t.Cleanup(func() { mockStore.AssertExpectations(t) })
+
+	const actorID = "bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb"
+	reg := &config.AccountRegistration{
+		ID:         "reg-id-1",
+		Status:     "pending",
+		ReviewedBy: &[]string{actorID}[0],
+	}
+	// Actor must equal the reviewer UUID.
+	mockStore.On("TransitionRegistrationStatus", ctx, reg, "pending",
+		mock.MatchedBy(func(a *string) bool { return a != nil && *a == actorID }),
+	).Return(nil)
+
+	err := mockStore.TransitionRegistrationStatus(ctx, reg, "pending", validUUIDPtrOrNil(reg.ReviewedBy))
+	require.NoError(t, err)
+}
+
+// TestHandler_TransitionRegistrationStatus_NonUUIDActorIsNil asserts that when
+// ReviewedBy is "admin-api-key" (API key session), the actor passed to
+// TransitionRegistrationStatus is nil so transitioned_by = NULL.
+func TestHandler_TransitionRegistrationStatus_NonUUIDActorIsNil(t *testing.T) {
+	ctx := context.Background()
+	mockStore := new(MockConfigStore)
+	t.Cleanup(func() { mockStore.AssertExpectations(t) })
+
+	nonUUID := "admin-api-key"
+	reg := &config.AccountRegistration{
+		ID:         "reg-id-2",
+		Status:     "pending",
+		ReviewedBy: &nonUUID,
+	}
+	// Non-UUID reviewer: actor must be nil.
+	mockStore.On("TransitionRegistrationStatus", ctx, reg, "pending", (*string)(nil)).Return(nil)
+
+	err := mockStore.TransitionRegistrationStatus(ctx, reg, "pending", validUUIDPtrOrNil(reg.ReviewedBy))
+	require.NoError(t, err)
+}

internal/api/handler_history.go

@@ -206,7 +206,7 @@ func (h *Handler) expireStaleExecutionsAsync(staleExecs []config.PurchaseExecuti
 	go func() {
 		ctx := context.Background()
 		for _, exec := range staleExecs {
-			_, err := h.config.TransitionExecutionStatus(ctx, exec.ExecutionID, []string{"pending", "notified"}, "expired")
+			_, err := h.config.TransitionExecutionStatus(ctx, exec.ExecutionID, []string{"pending", "notified"}, "expired", nil)
 			if err != nil {
 				logging.Warnf("history: async expire of execution %s failed: %v", exec.ExecutionID, err)
 			}
@@ -239,6 +239,25 @@ func (h *Handler) resolveUserEmails(ctx context.Context, executions []config.Pur
 	return out
 }
 
+// expireIfStale transitions a pending/notified execution to "expired" when
+// its ScheduledDate is older than approvalExpiryWindow. Returns the possibly-
+// updated execution. Transition failures are non-fatal — the row still
+// renders, just with its original status.
+func (h *Handler) expireIfStale(ctx context.Context, exec config.PurchaseExecution) config.PurchaseExecution {
+	if exec.Status != "pending" && exec.Status != "notified" {
+		return exec
+	}
+	if time.Since(exec.ScheduledDate) < approvalExpiryWindow {
+		return exec
+	}
+	updated, err := h.config.TransitionExecutionStatus(ctx, exec.ExecutionID, []string{"pending", "notified"}, "expired", nil)
+	if err != nil {
+		logging.Warnf("history: failed to expire execution %s: %v", exec.ExecutionID, err)
+		return exec
+	}
+	return *updated
+}
+
 // resolvePendingApproverEmail returns the notification email the approval
 // link was sent to (or would have been, if SES failed). Single-tenant
 // deployments share one value across every pending row, so this is looked up

internal/api/handler_history_test.go

@@ -242,8 +242,9 @@ func TestHandler_getHistory_ExpireIfStale(t *testing.T) {
 			Return([]config.PurchaseExecution{freshExec(), staleExec("pending")}, nil)
 		mockStore.On("GetGlobalConfig", ctx).Return(&config.GlobalConfig{NotificationEmail: &approverEmail}, nil)
 		// The goroutine uses context.Background(); context.Background() == ctx in
-		// this test, so the matcher fires correctly.
-		mockStore.On("TransitionExecutionStatus", mock.Anything, staleID, []string{"pending", "notified"}, "expired").
+		// this test, so the matcher fires correctly. The trailing mock.Anything
+		// matches the actor *string (nil for the system-initiated async expire).
+		mockStore.On("TransitionExecutionStatus", mock.Anything, staleID, []string{"pending", "notified"}, "expired", mock.Anything).
 			Run(waitForCall(done)).
 			Return(&expired, nil).Once()
 
@@ -263,7 +264,7 @@ func TestHandler_getHistory_ExpireIfStale(t *testing.T) {
 
 		// Exactly one Transition call, only for the stale row.
 		mockStore.AssertNumberOfCalls(t, "TransitionExecutionStatus", 1)
-		mockStore.AssertCalled(t, "TransitionExecutionStatus", mock.Anything, staleID, []string{"pending", "notified"}, "expired")
+		mockStore.AssertCalled(t, "TransitionExecutionStatus", mock.Anything, staleID, []string{"pending", "notified"}, "expired", mock.Anything)
 
 		historyResp := result.(HistoryResponse)
 		require.Len(t, historyResp.Purchases, 2, "both executions must render as history rows")
@@ -300,7 +301,7 @@ func TestHandler_getHistory_ExpireIfStale(t *testing.T) {
 		mockStore.On("GetExecutionsByStatuses", ctx, mock.Anything, mock.Anything).
 			Return([]config.PurchaseExecution{staleExec("notified")}, nil)
 		mockStore.On("GetGlobalConfig", ctx).Return(&config.GlobalConfig{NotificationEmail: &approverEmail}, nil)
-		mockStore.On("TransitionExecutionStatus", mock.Anything, staleID, []string{"pending", "notified"}, "expired").
+		mockStore.On("TransitionExecutionStatus", mock.Anything, staleID, []string{"pending", "notified"}, "expired", mock.Anything).
 			Run(waitForCall(done)).
 			Return(&expired, nil).Once()
 
@@ -337,7 +338,7 @@ func TestHandler_getHistory_ExpireIfStale(t *testing.T) {
 		mockStore.On("GetExecutionsByStatuses", ctx, mock.Anything, mock.Anything).
 			Return([]config.PurchaseExecution{staleExec("pending")}, nil)
 		mockStore.On("GetGlobalConfig", ctx).Return(&config.GlobalConfig{NotificationEmail: &approverEmail}, nil)
-		mockStore.On("TransitionExecutionStatus", mock.Anything, staleID, []string{"pending", "notified"}, "expired").
+		mockStore.On("TransitionExecutionStatus", mock.Anything, staleID, []string{"pending", "notified"}, "expired", mock.Anything).
 			Run(waitForCall(done)).
 			Return(nil, errors.New("simulated store failure")).Once()
 
@@ -540,6 +541,38 @@ func TestHandler_getHistory_ScopedUserSeesEmptyAccountRows(t *testing.T) {
 	assert.Equal(t, 1, resp.Summary.TotalPending)
 }
 
+// TestHandler_expireIfStale_SystemActorIsNil asserts that the expireIfStale
+// helper passes nil as the actor param to TransitionExecutionStatus.
+// expireIfStale is a system-initiated path (no human session), so transitioned_by
+// must be NULL on the affected row (issue #1009).
+func TestHandler_expireIfStale_SystemActorIsNil(t *testing.T) {
+	ctx := context.Background()
+	mockStore := new(MockConfigStore)
+	approverEmail := "ops@example.com"
+	t.Cleanup(func() { mockStore.AssertExpectations(t) })
+
+	staleID := "actor-nil-stale-exec"
+	expired := config.PurchaseExecution{ExecutionID: staleID, Status: "expired"}
+
+	mockStore.On("GetAllPurchaseHistory", ctx, 100).Return([]config.PurchaseHistoryRecord{}, nil)
+	mockStore.On("GetExecutionsByStatuses", ctx, mock.Anything, mock.Anything).
+		Return([]config.PurchaseExecution{{
+			ExecutionID:   staleID,
+			Status:        "pending",
+			ScheduledDate: time.Now().Add(-8 * 24 * time.Hour),
+		}}, nil)
+	mockStore.On("GetGlobalConfig", ctx).Return(&config.GlobalConfig{NotificationEmail: &approverEmail}, nil)
+	// System path: expireIfStale must pass nil actor so transitioned_by = NULL.
+	mockStore.On("TransitionExecutionStatus", ctx, staleID, []string{"pending", "notified"}, "expired",
+		(*string)(nil),
+	).Return(&expired, nil).Once()
+
+	mockAuth, req := adminHistoryReq(ctx)
+	handler := &Handler{auth: mockAuth, config: mockStore}
+	_, err := handler.getHistory(ctx, req, map[string]string{})
+	require.NoError(t, err)
+}
+
 // TestHandler_getHistory_PermissionDenied asserts that a non-admin user without
 // view:purchases gets 403 and never reaches the store.
 func TestHandler_getHistory_PermissionDenied(t *testing.T) {

internal/api/handler_per_account_perms_test.go

@@ -965,7 +965,7 @@ func TestPerAccountPerms_PlannedPurchase_AllowedAccountPlanSucceeds(t *testing.T
 	mockStore.On("GetExecutionByID", ctx, executionID).Return(&config.PurchaseExecution{
 		ExecutionID: executionID, PlanID: planID, Status: "pending",
 	}, nil)
-	mockStore.On("TransitionExecutionStatus", ctx, executionID, mock.Anything, "paused").
+	mockStore.On("TransitionExecutionStatus", ctx, executionID, mock.Anything, "paused", mock.Anything).
 		Return(transitoned, nil)
 
 	// Plan is associated with account A — within the scoped user's allowed set.
@@ -985,7 +985,7 @@ func TestPerAccountPerms_PlannedPurchase_AllowedAccountPlanSucceeds(t *testing.T
 	require.NoError(t, err, "scoped user must be able to pause an account-A execution")
 	require.NotNil(t, result)
 	assert.Equal(t, "paused", result.Status, "result must reflect the paused status")
-	mockStore.AssertCalled(t, "TransitionExecutionStatus", ctx, executionID, mock.Anything, "paused")
+	mockStore.AssertCalled(t, "TransitionExecutionStatus", ctx, executionID, mock.Anything, "paused", mock.Anything)
 }
 
 // ─── 10. GET /ri-exchange/instances ──────────────────────────────────────────

internal/api/handler_purchases.go

@@ -272,7 +272,7 @@ func (h *Handler) pausePlannedPurchase(ctx context.Context, req *events.LambdaFu
 	}
 
 	// Atomically transition to paused
-	if _, err := h.config.TransitionExecutionStatus(ctx, executionID, []string{"pending", "running"}, "paused"); err != nil {
+	if _, err := h.config.TransitionExecutionStatus(ctx, executionID, []string{"pending", "running"}, "paused", resolveCreatorUserID(session)); err != nil {
 		return nil, NewClientError(409, fmt.Sprintf("execution %s cannot be paused: %v", executionID, err))
 	}
 
@@ -296,7 +296,7 @@ func (h *Handler) resumePlannedPurchase(ctx context.Context, req *events.LambdaF
 	}
 
 	// Atomically transition from paused back to pending
-	if _, err := h.config.TransitionExecutionStatus(ctx, executionID, []string{"paused"}, "pending"); err != nil {
+	if _, err := h.config.TransitionExecutionStatus(ctx, executionID, []string{"paused"}, "pending", resolveCreatorUserID(session)); err != nil {
 		return nil, NewClientError(409, fmt.Sprintf("execution %s cannot be resumed: %v", executionID, err))
 	}
 
@@ -321,7 +321,7 @@ func (h *Handler) runPlannedPurchase(ctx context.Context, req *events.LambdaFunc
 
 	// Atomically transition to running — only one concurrent caller can succeed.
 	// TransitionExecutionStatus handles not-found and wrong-status cases.
-	if _, err := h.config.TransitionExecutionStatus(ctx, executionID, []string{"pending", "paused"}, "running"); err != nil {
+	if _, err := h.config.TransitionExecutionStatus(ctx, executionID, []string{"pending", "paused"}, "running", resolveCreatorUserID(session)); err != nil {
 		return nil, NewClientError(409, fmt.Sprintf("execution %s cannot be started: %v", executionID, err))
 	}
 
@@ -348,7 +348,7 @@ func (h *Handler) deletePlannedPurchase(ctx context.Context, req *events.LambdaF
 		return nil, err
 	}
 
-	cancelled, err := h.cancelOrRecoverExecution(ctx, executionID)
+	cancelled, err := h.cancelOrRecoverExecution(ctx, executionID, resolveCreatorUserID(session))
 	if err != nil {
 		return nil, err
 	}
@@ -371,8 +371,9 @@ func (h *Handler) deletePlannedPurchase(ctx context.Context, req *events.LambdaF
 // (ErrExecutionNotInExpectedStatus), it fetches the row instead so the caller
 // can still drive the plan-disable side-effect, keeping the operation
 // idempotent across retries.
-func (h *Handler) cancelOrRecoverExecution(ctx context.Context, executionID string) (*config.PurchaseExecution, error) {
-	cancelled, err := h.config.TransitionExecutionStatus(ctx, executionID, []string{"pending", "paused"}, "cancelled")
+// actor is the UUID of the user initiating the cancel (nil for system-initiated paths).
+func (h *Handler) cancelOrRecoverExecution(ctx context.Context, executionID string, actor *string) (*config.PurchaseExecution, error) {
+	cancelled, err := h.config.TransitionExecutionStatus(ctx, executionID, []string{"pending", "paused"}, "cancelled", actor)
 	if err == nil {
 		return cancelled, nil
 	}