chore: adding defensive checks to ics27 capability migrations (#2798) (…

…#2809)

* adding defensive checks and additional tests to ics27 capability migrations

* assert mock capabilities remain unchanged

* Update modules/apps/27-interchain-accounts/controller/migrations/v6/migrations_test.go

Co-authored-by: Carlos Rodriguez <[email protected]>

Co-authored-by: Carlos Rodriguez <[email protected]>
(cherry picked from commit c40d555)

Co-authored-by: Damian Nolan <[email protected]>

modules/apps/27-interchain-accounts/controller/migrations/v6/migrations.go

@@ -9,6 +9,7 @@ import (
 	capabilitytypes "github.com/cosmos/cosmos-sdk/x/capability/types"
 
 	controllertypes "github.com/cosmos/ibc-go/v6/modules/apps/27-interchain-accounts/controller/types"
+	host "github.com/cosmos/ibc-go/v6/modules/core/24-host"
 )
 
 // MigrateICS27ChannelCapability performs a search on a prefix store using the provided store key and module name.
@@ -32,6 +33,10 @@ func MigrateICS27ChannelCapability(
 		var owners capabilitytypes.CapabilityOwners
 		cdc.MustUnmarshal(iterator.Value(), &owners)
 
+		if !hasIBCOwner(owners.GetOwners()) {
+			continue
+		}
+
 		for _, owner := range owners.GetOwners() {
 			if owner.Module == module {
 				// remove the owner from the set
@@ -56,3 +61,17 @@ func MigrateICS27ChannelCapability(
 
 	return nil
 }
+
+func hasIBCOwner(owners []capabilitytypes.Owner) bool {
+	if len(owners) != 2 {
+		return false
+	}
+
+	for _, owner := range owners {
+		if owner.Module == host.ModuleName {
+			return true
+		}
+	}
+
+	return false
+}

modules/apps/27-interchain-accounts/controller/migrations/v6/migrations_test.go

@@ -93,6 +93,9 @@ func (suite *MigrationsTestSuite) TestMigrateICS27ChannelCapability() {
 	err := suite.SetupPath()
 	suite.Require().NoError(err)
 
+	// create additional capabilities to cover edge cases
+	suite.CreateMockCapabilities()
+
 	// create and claim a new capability with ibc/mock for "channel-1"
 	// note: suite.SetupPath() now claims the chanel capability using icacontroller for "channel-0"
 	capName := host.ChannelCapabilityPath(suite.path.EndpointA.ChannelConfig.PortID, channeltypes.FormatChannelIdentifier(1))
@@ -147,6 +150,38 @@ func (suite *MigrationsTestSuite) TestMigrateICS27ChannelCapability() {
 
 	isAuthenticated = suite.chainA.GetSimApp().ScopedICAControllerKeeper.AuthenticateCapability(suite.chainA.GetContext(), cap, capName)
 	suite.Require().True(isAuthenticated)
+
+	suite.AssertMockCapabiltiesUnchanged()
+}
+
+// CreateMockCapabilities creates an additional two capabilities used for testing purposes:
+// 1. A capability with a single owner
+// 2. A capability with two owners, neither of which is "ibc"
+func (suite *MigrationsTestSuite) CreateMockCapabilities() {
+	cap, err := suite.chainA.GetSimApp().ScopedIBCMockKeeper.NewCapability(suite.chainA.GetContext(), "mock_one")
+	suite.Require().NoError(err)
+	suite.Require().NotNil(cap)
+
+	cap, err = suite.chainA.GetSimApp().ScopedICAMockKeeper.NewCapability(suite.chainA.GetContext(), "mock_two")
+	suite.Require().NoError(err)
+	suite.Require().NotNil(cap)
+
+	err = suite.chainA.GetSimApp().ScopedIBCMockKeeper.ClaimCapability(suite.chainA.GetContext(), cap, "mock_two")
+	suite.Require().NoError(err)
+}
+
+// AssertMockCapabiltiesUnchanged authenticates the mock capabilities created at the start of the test to ensure they remain unchanged
+func (suite *MigrationsTestSuite) AssertMockCapabiltiesUnchanged() {
+	cap, found := suite.chainA.GetSimApp().ScopedIBCMockKeeper.GetCapability(suite.chainA.GetContext(), "mock_one")
+	suite.Require().True(found)
+	suite.Require().NotNil(cap)
+
+	cap, found = suite.chainA.GetSimApp().ScopedIBCMockKeeper.GetCapability(suite.chainA.GetContext(), "mock_two")
+	suite.Require().True(found)
+	suite.Require().NotNil(cap)
+
+	isAuthenticated := suite.chainA.GetSimApp().ScopedICAMockKeeper.AuthenticateCapability(suite.chainA.GetContext(), cap, "mock_two")
+	suite.Require().True(isAuthenticated)
 }
 
 // ResetMemstore removes all existing fwd and rev capability kv pairs and deletes `KeyMemInitialised` from the x/capability memstore.