Don't clear past lock/wipe (#41504)

<!-- Add the related story/sub-task/bug number, like Resolves #123, or
remove if NA -->
**Related issue:** Resolves #41190 

# Checklist for submitter

If some of the following don't apply, delete the relevant line.

- [x] Changes file added for user-visible changes in `changes/`,
`orbit/changes/` or `ee/fleetd-chrome/changes`.

## Testing

- [x] Added/updated automated tests
- [x] QA'd all new/changed functionality manually

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

## Bug Fixes
* Improved audit log accuracy when canceling pending lock or wipe
commands. The original activity record is now preserved, with the
cancellation tracked as a separate follow-up entry for better
visibility.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

changes/41190-audit-log-entries

@@ -0,0 +1 @@
+* Fixed a security issue where canceling a pending lock or wipe command permanently deleted the original `locked_host`/`wiped_host` activity from the audit log. The original activity is now preserved, and the subsequent cancellation activity serves as the follow-up record.

server/datastore/mysql/activities.go

@@ -561,7 +561,7 @@ func (ds *Datastore) cancelHostUpcomingActivity(ctx context.Context, tx sqlx.Ext
 	// if the activity is related to lock/wipe actions, clear the status for that
 	// action as it was canceled (note that lock/wipe is prevented at the service
 	// layer from being canceled if it was already activated).
-	if err := clearLockWipeForCanceledActivity(ctx, tx, hostID, executionID); err != nil {
+	if err := clearHostMDMActionsForCanceledLockWipe(ctx, tx, hostID, executionID); err != nil {
 		return nil, err
 	}
 
@@ -783,56 +783,19 @@ func cancelHostScriptUpcomingActivity(ctx context.Context, tx sqlx.ExtContext, a
 	}, nil
 }
 
-func clearLockWipeForCanceledActivity(ctx context.Context, tx sqlx.ExtContext, hostID uint, executionID string) error {
+func clearHostMDMActionsForCanceledLockWipe(ctx context.Context, tx sqlx.ExtContext, hostID uint, executionID string) error {
 	const clearLockStmt = `DELETE FROM host_mdm_actions WHERE host_id = ? AND lock_ref = ?`
-	resLock, err := tx.ExecContext(ctx, clearLockStmt, hostID, executionID)
+	_, err := tx.ExecContext(ctx, clearLockStmt, hostID, executionID)
 	if err != nil {
 		return ctxerr.Wrap(ctx, err, "delete host_mdm_actions for lock")
 	}
 
 	const clearWipeStmt = `DELETE FROM host_mdm_actions WHERE host_id = ? AND wipe_ref = ?`
-	resWipe, err := tx.ExecContext(ctx, clearWipeStmt, hostID, executionID)
+	_, err = tx.ExecContext(ctx, clearWipeStmt, hostID, executionID)
 	if err != nil {
 		return ctxerr.Wrap(ctx, err, "delete host_mdm_actions for wipe")
 	}
 
-	lockCnt, _ := resLock.RowsAffected()
-	wipeCnt, _ := resWipe.RowsAffected()
-	if lockCnt > 0 || wipeCnt > 0 {
-		// if it did delete host_mdm_actions, then it was a lock or wipe activity,
-		// we need to delete the "past" activity that gets created immediately
-		// when that command is queued.
-		actType := fleet.ActivityTypeLockedHost{}.ActivityName()
-		if wipeCnt > 0 {
-			actType = fleet.ActivityTypeWipedHost{}.ActivityName()
-		}
-
-		const findActStmt = `SELECT
-				id
-			FROM
-				activity_past
-				INNER JOIN activity_host_past ON (activity_host_past.activity_id = activity_past.id)
-			WHERE
-				activity_host_past.host_id = ? AND
-				activity_past.activity_type = ?
-			ORDER BY
-				activity_past.created_at DESC
-			LIMIT 1
-`
-		var activityID uint
-		if err := sqlx.GetContext(ctx, tx, &activityID, findActStmt, hostID, actType); err != nil {
-			if errors.Is(err, sql.ErrNoRows) {
-				// no activity to delete, nothing to do
-				return nil
-			}
-			return ctxerr.Wrap(ctx, err, "find past activity for lock/wipe")
-		}
-
-		const delStmt = `DELETE FROM activity_past WHERE id = ?`
-		if _, err := tx.ExecContext(ctx, delStmt, activityID); err != nil {
-			return ctxerr.Wrap(ctx, err, "delete past activity for lock/wipe")
-		}
-	}
 	return nil
 }
 

server/service/integration_mdm_test.go

@@ -19631,25 +19631,20 @@ func (s *integrationMDMTestSuite) TestCancelLockWipeUpcomingActivity() {
 	require.NotNil(t, getHostResp.Host.MDM.PendingAction)
 	require.EqualValues(t, fleet.PendingActionNone, *getHostResp.Host.MDM.PendingAction)
 
-	// past lock/wipe actions have been cleared
+	// original lock/wipe activities are preserved in the audit log, with the cancelation as the latest activity
 	var listAct listActivitiesResponse
 	s.DoJSON("GET", fmt.Sprintf("/api/latest/fleet/hosts/%d/activities", h3.ID), nil, http.StatusOK, &listAct)
-	// latest activity is the cancelation
-	require.True(t, len(listAct.Activities) > 0)
+	require.True(t, len(listAct.Activities) > 1)
 	require.Equal(t, fleet.ActivityTypeCanceledRunScript{}.ActivityName(), listAct.Activities[0].Type)
-	if len(listAct.Activities) > 1 {
-		require.NotEqual(t, lockActID, listAct.Activities[1].ID)
-		require.NotEqual(t, fleet.ActivityTypeLockedHost{}.ActivityName(), listAct.Activities[1].Type)
-	}
+	require.Equal(t, lockActID, listAct.Activities[1].ID)
+	require.Equal(t, fleet.ActivityTypeLockedHost{}.ActivityName(), listAct.Activities[1].Type)
 
 	listAct = listActivitiesResponse{}
 	s.DoJSON("GET", fmt.Sprintf("/api/latest/fleet/hosts/%d/activities", h4.ID), nil, http.StatusOK, &listAct)
-	require.True(t, len(listAct.Activities) > 0)
+	require.True(t, len(listAct.Activities) > 1)
 	require.Equal(t, fleet.ActivityTypeCanceledRunScript{}.ActivityName(), listAct.Activities[0].Type)
-	if len(listAct.Activities) > 1 {
-		require.NotEqual(t, wipeActID, listAct.Activities[1].ID)
-		require.NotEqual(t, fleet.ActivityTypeWipedHost{}.ActivityName(), listAct.Activities[1].Type)
-	}
+	require.Equal(t, wipeActID, listAct.Activities[1].ID)
+	require.Equal(t, fleet.ActivityTypeWipedHost{}.ActivityName(), listAct.Activities[1].Type)
 }
 
 func (s *integrationMDMTestSuite) TestSoftwareCategories() {