From 5ceb55be036313778f75701e21173df4667fc200 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Juan=20Jos=C3=A9=20Mata?= <juanjo.mata@gmail.com>
Date: Tue, 12 May 2026 12:17:00 +0200
Subject: [PATCH] Scope SnapTrade orphan cleanup to current family (#1769)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* Scope SnapTrade orphan cleanup to current family

Restrict orphaned user listing and deletion to SnapTrade user IDs that belong to the current family namespace. Add model tests to prevent cross-family enumeration/deletion regressions.

* Update test/models/snaptrade_item_test.rb

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Signed-off-by: Juan José Mata <jjmata@jjmata.com>

* test: fix snaptrade orphaned users assertion

* style: fix snaptrade test array spacing

---------

Signed-off-by: Juan José Mata <jjmata@jjmata.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: KiloClaw <kiloclaw@openclaw.ai>
---
 app/models/snaptrade_item/provided.rb |  3 ++-
 test/models/snaptrade_item_test.rb    | 38 +++++++++++++++++++++++++++
 2 files changed, 40 insertions(+), 1 deletion(-)

diff --git a/app/models/snaptrade_item/provided.rb b/app/models/snaptrade_item/provided.rb
index 5ffb43906..c8d104f43 100644
--- a/app/models/snaptrade_item/provided.rb
+++ b/app/models/snaptrade_item/provided.rb
@@ -160,13 +160,14 @@ module SnaptradeItem::Provided
     return [] unless credentials_configured? && user_registered?
 
     all_users = list_all_users
-    all_users.reject { |uid| uid == snaptrade_user_id }
+    all_users.select { |uid| uid != snaptrade_user_id && uid.start_with?("family_#{family_id}_") }
   end
 
   # Delete an orphaned SnapTrade user and all their connections
   def delete_orphaned_user(user_id)
     return false unless credentials_configured?
     return false if user_id == snaptrade_user_id # Don't delete current user
+    return false unless user_id.start_with?("family_#{family_id}_")
 
     snaptrade_provider.delete_user(user_id: user_id)
     true
diff --git a/test/models/snaptrade_item_test.rb b/test/models/snaptrade_item_test.rb
index 79e6c6689..6b31c9895 100644
--- a/test/models/snaptrade_item_test.rb
+++ b/test/models/snaptrade_item_test.rb
@@ -75,4 +75,42 @@ class SnaptradeItemTest < ActiveSupport::TestCase
     provider = item.snaptrade_provider
     assert_instance_of Provider::Snaptrade, provider
   end
+
+  test "orphaned_users only includes users for the same family" do
+    item = SnaptradeItem.new(
+      family: @family,
+      name: "Test",
+      client_id: "test",
+      consumer_key: "test",
+      snaptrade_user_id: "family_#{@family.id}_111",
+      snaptrade_user_secret: "secret"
+    )
+
+    item.stubs(:list_all_users).returns([
+      "family_#{@family.id}_111",
+      "family_#{@family.id}_222",
+      "family_999_333",
+      "legacy_user_444"
+    ])
+
+    assert_equal([ "family_#{@family.id}_222" ], item.orphaned_users)
+  end
+
+  test "delete_orphaned_user rejects users outside the current family namespace" do
+    item = SnaptradeItem.new(
+      family: @family,
+      name: "Test",
+      client_id: "test",
+      consumer_key: "test",
+      snaptrade_user_id: "family_#{@family.id}_111",
+      snaptrade_user_secret: "secret"
+    )
+
+    provider = mock
+    provider.expects(:delete_user).never
+    item.stubs(:snaptrade_provider).returns(provider)
+
+    assert_not item.delete_orphaned_user("family_999_222")
+    assert_not item.delete_orphaned_user("legacy_user_333")
+  end
 end