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

internal/analytics/collector_test.go

@@ -273,7 +273,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
 }
 
@@ -301,7 +301,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 {
@@ -386,7 +386,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,96 @@ 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 drives the real
+// rejectRegistration handler and asserts that TransitionRegistrationStatus is
+// called with a non-nil actor equal to the reviewing admin session's UUID (the
+// common human-reviewed path). Exercising the handler (not the store directly)
+// keeps the test honest: it fails if the handler ever stops deriving the actor
+// from the session and threading it through (CR feedback on PR #1011).
+func TestHandler_TransitionRegistrationStatus_ActorStamped(t *testing.T) {
+	ctx := context.Background()
+	const regID = "11111111-1111-1111-1111-111111111111"
+	const actorID = "bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb"
+
+	mockStore := new(MockConfigStore)
+	mockAuth := new(MockAuthService)
+	t.Cleanup(func() { mockStore.AssertExpectations(t) })
+	t.Cleanup(func() { mockAuth.AssertExpectations(t) })
+
+	reg := &config.AccountRegistration{ID: regID, Status: "pending"}
+	mockStore.On("GetAccountRegistration", ctx, regID).Return(reg, nil)
+	// setReviewMetadata stamps reg.ReviewedBy = session.UserID, so the actor
+	// threaded into the store must equal the session UUID.
+	mockStore.On("TransitionRegistrationStatus", ctx, reg, "pending",
+		mock.MatchedBy(func(a *string) bool { return a != nil && *a == actorID }),
+	).Return(nil)
+
+	mockAuth.On("ValidateSession", ctx, "sess-tok").Return(&Session{UserID: actorID, Email: "admin@example.com"}, nil)
+	mockAuth.grantAdmin()
+
+	handler := &Handler{config: mockStore, auth: mockAuth}
+	req := &events.LambdaFunctionURLRequest{
+		Headers: map[string]string{"authorization": "Bearer sess-tok"},
+	}
+	_, err := handler.rejectRegistration(ctx, req, regID)
+	require.NoError(t, err)
+}
+
+// TestHandler_TransitionRegistrationStatus_NonUUIDActorIsNil drives the real
+// rejectRegistration handler via the admin-API-key path, where requireAdmin
+// returns a session whose UserID is the literal "admin-api-key" (not a UUID
+// FK). The handler must pass nil as the actor so transitioned_by = NULL.
+func TestHandler_TransitionRegistrationStatus_NonUUIDActorIsNil(t *testing.T) {
+	ctx := context.Background()
+	const regID = "22222222-2222-2222-2222-222222222222"
+
+	mockStore := new(MockConfigStore)
+	t.Cleanup(func() { mockStore.AssertExpectations(t) })
+
+	reg := &config.AccountRegistration{ID: regID, Status: "pending"}
+	mockStore.On("GetAccountRegistration", ctx, regID).Return(reg, nil)
+	// API-key reviewer ("admin-api-key") is not a UUID: actor must be nil.
+	mockStore.On("TransitionRegistrationStatus", ctx, reg, "pending", (*string)(nil)).Return(nil)
+
+	handler := &Handler{config: mockStore, apiKey: "admin-secret"}
+	req := &events.LambdaFunctionURLRequest{
+		Headers: map[string]string{"x-api-key": "admin-secret"},
+	}
+	_, err := handler.rejectRegistration(ctx, req, regID)
+	require.NoError(t, err)
+}

internal/api/handler_history.go

@@ -210,7 +210,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)
 			}
@@ -243,6 +243,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()
 
@@ -431,7 +432,7 @@ func TestHandler_getHistory_GetIsReadOnly(t *testing.T) {
 	mockStore.On("GetAllPurchaseHistory", ctx, 100).Return([]config.PurchaseHistoryRecord{}, nil)
 	mockStore.On("GetExecutionsByStatuses", ctx, mock.Anything, mock.Anything).Return([]config.PurchaseExecution{stale}, nil)
 	mockStore.On("GetGlobalConfig", ctx).Return(&config.GlobalConfig{NotificationEmail: &approverEmail}, nil)
-	mockStore.On("TransitionExecutionStatus", mock.Anything, "stale-ro-exec", []string{"pending", "notified"}, "expired").
+	mockStore.On("TransitionExecutionStatus", mock.Anything, "stale-ro-exec", []string{"pending", "notified"}, "expired", mock.Anything).
 		Run(func(_ mock.Arguments) {
 			close(transitionCalled) // signal that the goroutine reached the transition
 			<-gate                  // block until the test releases it
@@ -540,6 +541,50 @@ func TestHandler_getHistory_ScopedUserSeesEmptyAccountRows(t *testing.T) {
 	assert.Equal(t, 1, resp.Summary.TotalPending)
 }
 
+// TestHandler_expireStaleExecutionsAsync_SystemActorIsNil asserts that the
+// async stale-expire sweep passes nil as the actor param to
+// TransitionExecutionStatus. Expiry is a system-initiated path (no human
+// session), so transitioned_by must be NULL on the affected row (issue #1009).
+// The transition fires in a background goroutine (issue #1032: GET is a pure
+// read), so the test blocks on a done channel rather than asserting
+// synchronously.
+func TestHandler_expireStaleExecutionsAsync_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"}
+	done := make(chan struct{})
+
+	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: the async expire must pass nil actor so transitioned_by =
+	// NULL. The (*string)(nil) literal is the contract under test. The
+	// goroutine uses context.Background(); use mock.Anything for ctx.
+	mockStore.On("TransitionExecutionStatus", mock.Anything, staleID, []string{"pending", "notified"}, "expired",
+		(*string)(nil),
+	).Run(func(_ mock.Arguments) { close(done) }).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)
+
+	select {
+	case <-done:
+	case <-time.After(5 * time.Second):
+		t.Fatal("expire goroutine did not call TransitionExecutionStatus within 5s")
+	}
+}
+
 // 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 ──────────────────────────────────────────