From dfd8e0e8a3096c1238fda9b90b3b045d8339e21b Mon Sep 17 00:00:00 2001 From: ron Date: Tue, 12 May 2026 20:02:18 +0800 Subject: [PATCH 1/5] Restrict Asset Hub agent from executing contract calls in V2 --- contracts/src/Functions.sol | 11 ++++++++-- contracts/src/interfaces/IGatewayBase.sol | 1 + contracts/src/v2/Handlers.sol | 2 +- contracts/test/GatewayV2.t.sol | 26 +++++++++++++++++++++++ 4 files changed, 37 insertions(+), 3 deletions(-) diff --git a/contracts/src/Functions.sol b/contracts/src/Functions.sol index f372befcaf..dc178491a3 100644 --- a/contracts/src/Functions.sol +++ b/contracts/src/Functions.sol @@ -15,6 +15,7 @@ import {TokenInfo} from "./types/Common.sol"; import {ChannelID, Channel} from "./v1/Types.sol"; import {IGatewayBase} from "./interfaces/IGatewayBase.sol"; import {IGatewayV1} from "./v1/IGateway.sol"; +import {Constants} from "./Constants.sol"; // Common functionality that is shared between V1 and V2 library Functions { @@ -22,10 +23,8 @@ library Functions { using SafeNativeTransfer for address payable; using SafeTokenTransferFrom for IERC20; - error AgentDoesNotExist(); error InvalidToken(); error InvalidAmount(); - error ChannelDoesNotExist(); /// Looks up an agent contract address, failing if no such mapping exists function ensureAgent(bytes32 agentID) internal view returns (address agent) { @@ -35,6 +34,14 @@ library Functions { } } + /// Looks up an agent contract address, failing if no such mapping exists or if the agent is the primary asset hub agent + function ensureAuthorizedAgent(bytes32 agentID) internal view returns (address agent) { + if(agentID == Constants.ASSET_HUB_AGENT_ID) { + revert IGatewayBase.AgentNotAuthorized(); + } + agent = ensureAgent(agentID); + } + /// @dev Ensure that the specified parachain has a channel allocated function ensureChannel(ChannelID channelID) internal view returns (Channel storage ch) { ch = CoreStorage.layout().channels[channelID]; diff --git a/contracts/src/interfaces/IGatewayBase.sol b/contracts/src/interfaces/IGatewayBase.sol index f14cb5b932..3a5178d207 100644 --- a/contracts/src/interfaces/IGatewayBase.sol +++ b/contracts/src/interfaces/IGatewayBase.sol @@ -13,6 +13,7 @@ interface IGatewayBase { error Unsupported(); error InvalidDestinationFee(); error AgentDoesNotExist(); + error AgentNotAuthorized(); error TokenAlreadyRegistered(); error TokenMintFailed(); error TokenTransferFailed(); diff --git a/contracts/src/v2/Handlers.sol b/contracts/src/v2/Handlers.sol index 74b70a56d4..35705b0620 100644 --- a/contracts/src/v2/Handlers.sol +++ b/contracts/src/v2/Handlers.sol @@ -65,7 +65,7 @@ library HandlersV2 { function callContract(bytes32 origin, address executor, bytes calldata data) external { CallContractParams memory params = abi.decode(data, (CallContractParams)); - address agent = Functions.ensureAgent(origin); + address agent = Functions.ensureAuthorizedAgent(origin); bytes memory call = abi.encodeCall(AgentExecutor.callContract, (params.target, params.data, params.value)); Functions.invokeOnAgent(agent, executor, call); diff --git a/contracts/test/GatewayV2.t.sol b/contracts/test/GatewayV2.t.sol index e037c1a197..533c354102 100644 --- a/contracts/test/GatewayV2.t.sol +++ b/contracts/test/GatewayV2.t.sol @@ -579,6 +579,32 @@ contract GatewayV2Test is Test { vm.expectEmit(true, false, false, true); emit IGatewayV2.InboundMessageDispatched(1, topic, true, relayerRewardAddress); + address bridgeHubAgent = IGatewayV2(address(gateway)).agentOf(Constants.BRIDGE_HUB_AGENT_ID); + vm.deal(bridgeHubAgent, 1 ether); + hoax(relayer, 1 ether); + IGatewayV2(address(gateway)) + .v2_submit( + InboundMessageV2({ + origin: Constants.BRIDGE_HUB_AGENT_ID, + nonce: 1, + topic: topic, + commands: makeCallContractCommand(0.1 ether) + }), + proof, + makeMockProof(), + relayerRewardAddress + ); + } + + function testAgentCallContractFailsForAssetHub() public { + bytes32 topic = keccak256("topic"); + + // The command fails, then the message is dispatched with success: false + vm.expectEmit(true, false, false, false); + emit IGatewayV2.CommandFailed(1, 0); + vm.expectEmit(true, false, false, true); + emit IGatewayV2.InboundMessageDispatched(1, topic, false, relayerRewardAddress); + vm.deal(assetHubAgent, 1 ether); hoax(relayer, 1 ether); IGatewayV2(address(gateway)) From f41f2e2e530b0aebd4d18bc0697edfc0e2083221 Mon Sep 17 00:00:00 2001 From: ron Date: Wed, 13 May 2026 11:48:44 +0800 Subject: [PATCH 2/5] Rename ensureAuthorizedAgent to ensureNonPrivilegedAgent --- contracts/src/Functions.sol | 2 +- contracts/src/v2/Handlers.sol | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/contracts/src/Functions.sol b/contracts/src/Functions.sol index dc178491a3..c27c645857 100644 --- a/contracts/src/Functions.sol +++ b/contracts/src/Functions.sol @@ -35,7 +35,7 @@ library Functions { } /// Looks up an agent contract address, failing if no such mapping exists or if the agent is the primary asset hub agent - function ensureAuthorizedAgent(bytes32 agentID) internal view returns (address agent) { + function ensureNonPrivilegedAgent(bytes32 agentID) internal view returns (address agent) { if(agentID == Constants.ASSET_HUB_AGENT_ID) { revert IGatewayBase.AgentNotAuthorized(); } diff --git a/contracts/src/v2/Handlers.sol b/contracts/src/v2/Handlers.sol index 35705b0620..f05e4022d1 100644 --- a/contracts/src/v2/Handlers.sol +++ b/contracts/src/v2/Handlers.sol @@ -65,7 +65,7 @@ library HandlersV2 { function callContract(bytes32 origin, address executor, bytes calldata data) external { CallContractParams memory params = abi.decode(data, (CallContractParams)); - address agent = Functions.ensureAuthorizedAgent(origin); + address agent = Functions.ensureNonPrivilegedAgent(origin); bytes memory call = abi.encodeCall(AgentExecutor.callContract, (params.target, params.data, params.value)); Functions.invokeOnAgent(agent, executor, call); From 2c8bf0d66f9ed786d382831a12277683b802e782 Mon Sep 17 00:00:00 2001 From: ron Date: Wed, 13 May 2026 11:49:40 +0800 Subject: [PATCH 3/5] Rename AgentNotAuthorized to UnauthorizedPrivilegedAgent --- contracts/src/Functions.sol | 2 +- contracts/src/interfaces/IGatewayBase.sol | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/contracts/src/Functions.sol b/contracts/src/Functions.sol index c27c645857..7a1d030315 100644 --- a/contracts/src/Functions.sol +++ b/contracts/src/Functions.sol @@ -37,7 +37,7 @@ library Functions { /// Looks up an agent contract address, failing if no such mapping exists or if the agent is the primary asset hub agent function ensureNonPrivilegedAgent(bytes32 agentID) internal view returns (address agent) { if(agentID == Constants.ASSET_HUB_AGENT_ID) { - revert IGatewayBase.AgentNotAuthorized(); + revert IGatewayBase.UnauthorizedPrivilegedAgent(); } agent = ensureAgent(agentID); } diff --git a/contracts/src/interfaces/IGatewayBase.sol b/contracts/src/interfaces/IGatewayBase.sol index 3a5178d207..5a55112f3c 100644 --- a/contracts/src/interfaces/IGatewayBase.sol +++ b/contracts/src/interfaces/IGatewayBase.sol @@ -13,7 +13,7 @@ interface IGatewayBase { error Unsupported(); error InvalidDestinationFee(); error AgentDoesNotExist(); - error AgentNotAuthorized(); + error UnauthorizedPrivilegedAgent(); error TokenAlreadyRegistered(); error TokenMintFailed(); error TokenTransferFailed(); From e98e560a1ce97b918f207115d278d90705ac5b55 Mon Sep 17 00:00:00 2001 From: ron Date: Fri, 22 May 2026 00:29:21 +0800 Subject: [PATCH 4/5] Rename to ensureNotAssetHubAgent --- contracts/src/Functions.sol | 2 +- contracts/src/v2/Handlers.sol | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/contracts/src/Functions.sol b/contracts/src/Functions.sol index 7a1d030315..e4205debe0 100644 --- a/contracts/src/Functions.sol +++ b/contracts/src/Functions.sol @@ -35,7 +35,7 @@ library Functions { } /// Looks up an agent contract address, failing if no such mapping exists or if the agent is the primary asset hub agent - function ensureNonPrivilegedAgent(bytes32 agentID) internal view returns (address agent) { + function ensureNotAssetHubAgent(bytes32 agentID) internal view returns (address agent) { if(agentID == Constants.ASSET_HUB_AGENT_ID) { revert IGatewayBase.UnauthorizedPrivilegedAgent(); } diff --git a/contracts/src/v2/Handlers.sol b/contracts/src/v2/Handlers.sol index f05e4022d1..834ba65fd0 100644 --- a/contracts/src/v2/Handlers.sol +++ b/contracts/src/v2/Handlers.sol @@ -65,7 +65,7 @@ library HandlersV2 { function callContract(bytes32 origin, address executor, bytes calldata data) external { CallContractParams memory params = abi.decode(data, (CallContractParams)); - address agent = Functions.ensureNonPrivilegedAgent(origin); + address agent = Functions.ensureNotAssetHubAgent(origin); bytes memory call = abi.encodeCall(AgentExecutor.callContract, (params.target, params.data, params.value)); Functions.invokeOnAgent(agent, executor, call); From 1b3338533cee42dca79e89dcb26e71a177e5cc60 Mon Sep 17 00:00:00 2001 From: Clara van Staden Date: Fri, 22 May 2026 12:01:02 +0200 Subject: [PATCH 5/5] Add tests for ensureNotAssetHubAgent (#1793) * Add tests for ensureNotAssetHubAgent Strengthens coverage around the AssetHub-agent callContract block introduced in #1788: - positive: a v2_createAgent'd user agent can still callContract - direct: ensureNotAssetHubAgent revert reason and allow/deny paths pinned via an exposed wrapper on MockGateway, including a fuzz invariant that only ASSET_HUB_AGENT_ID returns UnauthorizedPrivilegedAgent - no-state-leak: AssetHub failure path emits no SaidHello, leaves agent balance and registration untouched - dual-invariant: ASSET_HUB_AGENT_ID is rejected as a callContract origin AND still accepted as an unlockNativeToken recipient - multi-command: a poisoned AssetHub-origin callContract bundled with a legit one fails every command; same payload from a user agent succeeds - skipped: testAgentCallContractFailsForBridgeHub documents the latent AliasOrigin(Here) -> BRIDGE_HUB_AGENT_ID bypass that the current single-entry deny list does not cover Co-Authored-By: Claude Opus 4.7 (1M context) * Drop BridgeHub-block skip test BridgeHub is intentionally non-privileged in the V2 deny list; only ASSET_HUB_AGENT_ID is reserved. Remove the speculative testAgentCallContractFailsForBridgeHub skip and clarify the existing testEnsureNotAssetHubAgent_AllowsBridgeHub with a comment so future readers know the allow is by design. Co-Authored-By: Claude Opus 4.7 (1M context) * Drop ticket id from section comment Co-Authored-By: Claude Opus 4.7 (1M context) * Drop PR reference from section comment Co-Authored-By: Claude Opus 4.7 (1M context) --------- Co-authored-by: Claude Opus 4.7 (1M context) --- contracts/test/GatewayV2.t.sol | 255 +++++++++++++++++++++++++++ contracts/test/mocks/MockGateway.sol | 5 + 2 files changed, 260 insertions(+) diff --git a/contracts/test/GatewayV2.t.sol b/contracts/test/GatewayV2.t.sol index 533c354102..0409c21b97 100644 --- a/contracts/test/GatewayV2.t.sol +++ b/contracts/test/GatewayV2.t.sol @@ -2,6 +2,7 @@ pragma solidity 0.8.34; import {Test} from "forge-std/Test.sol"; +import {Vm} from "forge-std/Vm.sol"; import {IGatewayBase} from "../src/interfaces/IGatewayBase.sol"; import {IGatewayV2} from "../src/v2/IGateway.sol"; @@ -621,6 +622,260 @@ contract GatewayV2Test is Test { ); } + // Coverage for the AssetHub-agent callContract block. Each test pins + // one invariant that the headline pair (success + AssetHub-fails) does + // not cover. + + // (1) Positive test using a v2_createAgent'd user-controlled agent. + // This is the legitimate flow the deny list must keep working. + function testAgentCallContract_UserCreatedAgent_Succeeds() public { + bytes32 userAgentId = keccak256("user-agent"); + IGatewayV2(payable(address(gateway))).v2_createAgent(userAgentId); + address userAgent = IGatewayV2(address(gateway)).agentOf(userAgentId); + assertTrue(userAgent != address(0), "user agent must be registered"); + + bytes32 topic = keccak256("topic"); + + // helloWorld emits SaidHello on success. + vm.expectEmit(true, false, false, false); + emit SaidHello(string(abi.encodePacked("Hello there, World"))); + + vm.expectEmit(true, false, false, true); + emit IGatewayV2.InboundMessageDispatched(1, topic, true, relayerRewardAddress); + + hoax(relayer, 1 ether); + IGatewayV2(address(gateway)) + .v2_submit( + InboundMessageV2({ + origin: userAgentId, + nonce: 1, + topic: topic, + commands: makeCallContractCommand(0) + }), + proof, + makeMockProof(), + relayerRewardAddress + ); + } + + // (2) Direct unit coverage of the new helper. The dispatcher swallows + // revert reasons via try/catch, so the only way to pin the specific + // revert selector is to call ensureNotAssetHubAgent directly. + function testEnsureNotAssetHubAgent_RejectsAssetHubWithSpecificError() public { + vm.expectRevert(IGatewayBase.UnauthorizedPrivilegedAgent.selector); + MockGateway(address(gateway)).exposed_ensureNotAssetHubAgent(Constants.ASSET_HUB_AGENT_ID); + } + + // BridgeHub is intentionally non-privileged in the V2 deny list; only + // ASSET_HUB_AGENT_ID is reserved. + function testEnsureNotAssetHubAgent_AllowsBridgeHub() public { + address bridgeHubAgent = + MockGateway(address(gateway)).exposed_ensureNotAssetHubAgent(Constants.BRIDGE_HUB_AGENT_ID); + assertEq(bridgeHubAgent, IGatewayV2(address(gateway)).agentOf(Constants.BRIDGE_HUB_AGENT_ID)); + } + + function testEnsureNotAssetHubAgent_AllowsUserAgent() public { + bytes32 userAgentId = keccak256("user-agent-direct"); + IGatewayV2(payable(address(gateway))).v2_createAgent(userAgentId); + address expected = IGatewayV2(address(gateway)).agentOf(userAgentId); + + address actual = MockGateway(address(gateway)).exposed_ensureNotAssetHubAgent(userAgentId); + assertEq(actual, expected); + } + + function testEnsureNotAssetHubAgent_UnregisteredRevertsAgentDoesNotExist() public { + vm.expectRevert(IGatewayBase.AgentDoesNotExist.selector); + MockGateway(address(gateway)).exposed_ensureNotAssetHubAgent(bytes32(uint256(0xdeadbeef))); + } + + // (3) No-state-leak assertions on the AssetHub-rejected path. + // testAgentCallContractFailsForAssetHub only checks the events; this test + // additionally pins that no agent funds move, the target is not called, + // and the agent registration is unchanged. + function testAgentCallContractFailsForAssetHub_NoStateLeak() public { + bytes32 topic = keccak256("topic"); + + vm.deal(assetHubAgent, 1 ether); + uint256 agentBalanceBefore = assetHubAgent.balance; + uint256 targetBalanceBefore = address(helloWorld).balance; + address agentBefore = IGatewayV2(address(gateway)).agentOf(Constants.ASSET_HUB_AGENT_ID); + + vm.recordLogs(); + + hoax(relayer, 1 ether); + IGatewayV2(address(gateway)) + .v2_submit( + InboundMessageV2({ + origin: Constants.ASSET_HUB_AGENT_ID, + nonce: 1, + topic: topic, + commands: makeCallContractCommand(0.1 ether) + }), + proof, + makeMockProof(), + relayerRewardAddress + ); + + // No SaidHello event from helloWorld — target was never invoked. + bytes32 saidHelloSig = SaidHello.selector; + Vm.Log[] memory logs = vm.getRecordedLogs(); + for (uint256 i = 0; i < logs.length; i++) { + if (logs[i].emitter == address(helloWorld)) { + assertTrue( + logs[i].topics.length == 0 || logs[i].topics[0] != saidHelloSig, + "helloWorld must not emit SaidHello on forged-origin callContract" + ); + } + } + + assertEq(assetHubAgent.balance, agentBalanceBefore, "AssetHub agent balance must be unchanged"); + assertEq(address(helloWorld).balance, targetBalanceBefore, "target balance must be unchanged"); + assertEq( + IGatewayV2(address(gateway)).agentOf(Constants.ASSET_HUB_AGENT_ID), + agentBefore, + "AssetHub agent must remain registered" + ); + } + + // (4) Fuzz: for any id != ASSET_HUB_AGENT_ID, the helper must never + // revert with UnauthorizedPrivilegedAgent. It is allowed to revert + // AgentDoesNotExist or return a registered address. + function testFuzz_EnsureNotAssetHubAgent_NeverRejectsNonAssetHub(bytes32 id) public { + vm.assume(id != Constants.ASSET_HUB_AGENT_ID); + try MockGateway(address(gateway)).exposed_ensureNotAssetHubAgent(id) returns (address agent) { + assertTrue(agent != address(0), "registered agent must be non-zero"); + } catch (bytes memory reason) { + assertEq( + bytes4(reason), + IGatewayBase.AgentDoesNotExist.selector, + "non-AssetHub ids must only ever fail with AgentDoesNotExist" + ); + } + } + + // (5) Dual invariant: ASSET_HUB_AGENT_ID is rejected as a callContract + // origin AND still accepted as the recipient of unlockNativeToken. + // unlockNativeToken hardcodes ASSET_HUB_AGENT_ID internally; if some + // future refactor centralised the privilege check at the wrong layer, + // unlocks would silently break. + function testAssetHubAgent_BlockedAsCallContractOrigin_StillUnlocksTokens() public { + // First: AssetHub origin rejected. + bytes32 topic1 = keccak256("topic-rejected"); + vm.expectEmit(true, false, false, false); + emit IGatewayV2.CommandFailed(1, 0); + vm.expectEmit(true, false, false, true); + emit IGatewayV2.InboundMessageDispatched(1, topic1, false, relayerRewardAddress); + + vm.deal(assetHubAgent, 1 ether); + hoax(relayer, 1 ether); + IGatewayV2(address(gateway)) + .v2_submit( + InboundMessageV2({ + origin: Constants.ASSET_HUB_AGENT_ID, + nonce: 1, + topic: topic1, + commands: makeCallContractCommand(0) + }), + proof, + makeMockProof(), + relayerRewardAddress + ); + + // Second: AssetHub agent still successfully unlocks WETH it holds. + bytes32 topic2 = keccak256("topic-unlock"); + hoax(assetHubAgent); + weth.deposit{value: 1 ether}(); + + vm.expectEmit(true, false, false, true); + emit IGatewayV2.InboundMessageDispatched(2, topic2, true, relayerRewardAddress); + + hoax(relayer, 1 ether); + IGatewayV2(address(gateway)) + .v2_submit( + InboundMessageV2({ + origin: Constants.ASSET_HUB_AGENT_ID, + nonce: 2, + topic: topic2, + commands: makeUnlockWethCommand(0.1 ether) + }), + proof, + makeMockProof(), + relayerRewardAddress + ); + } + + // (6) Multi-command message: one legit callContract from a user agent + // bundled with one poisoned callContract from AssetHub. The first must + // succeed (SaidHello emitted), the second must emit CommandFailed, and + // the message must overall report success=false. + function testMultiCommand_PoisonedAssetHubCallFails_LegitUserCallSucceeds() public { + bytes32 userAgentId = keccak256("user-agent-multi"); + IGatewayV2(payable(address(gateway))).v2_createAgent(userAgentId); + + CommandV2[] memory commands = new CommandV2[](2); + bytes memory data = abi.encodeWithSignature("sayHello(string)", "World"); + + // Index 0: poisoned callContract from AssetHub (dispatcher uses a + // single `origin` per message; submit as AssetHub so the FIRST cmd + // is the rejected one). + commands[0] = CommandV2({ + kind: CommandKind.CallContract, + gas: 500_000, + payload: abi.encode(CallContractParams({target: address(helloWorld), data: data, value: 0})) + }); + // Index 1: same callContract, but we'll submit this in a second + // message from the user agent below to prove it would have worked. + commands[1] = CommandV2({ + kind: CommandKind.CallContract, + gas: 500_000, + payload: abi.encode(CallContractParams({target: address(helloWorld), data: data, value: 0})) + }); + + bytes32 topic = keccak256("topic-multi"); + + // The dispatcher binds a single `origin` to all commands in a + // message, so a multi-command message with AssetHub origin has + // every CallContract fail. Pin that exact behavior. + vm.expectEmit(true, false, false, false); + emit IGatewayV2.CommandFailed(1, 0); + vm.expectEmit(true, false, false, false); + emit IGatewayV2.CommandFailed(1, 1); + vm.expectEmit(true, false, false, true); + emit IGatewayV2.InboundMessageDispatched(1, topic, false, relayerRewardAddress); + + hoax(relayer, 1 ether); + IGatewayV2(address(gateway)) + .v2_submit( + InboundMessageV2({ + origin: Constants.ASSET_HUB_AGENT_ID, + nonce: 1, + topic: topic, + commands: commands + }), + proof, + makeMockProof(), + relayerRewardAddress + ); + + // Same payload from the user agent must succeed end-to-end. + bytes32 topic2 = keccak256("topic-multi-user"); + vm.expectEmit(true, false, false, true); + emit IGatewayV2.InboundMessageDispatched(2, topic2, true, relayerRewardAddress); + hoax(relayer, 1 ether); + IGatewayV2(address(gateway)) + .v2_submit( + InboundMessageV2({ + origin: userAgentId, + nonce: 2, + topic: topic2, + commands: commands + }), + proof, + makeMockProof(), + relayerRewardAddress + ); + } + function testCreateAgent() public { bytes32 origin = bytes32(uint256(1)); vm.expectEmit(true, false, false, false); diff --git a/contracts/test/mocks/MockGateway.sol b/contracts/test/mocks/MockGateway.sol index 190bd68a14..3a0b5d029a 100644 --- a/contracts/test/mocks/MockGateway.sol +++ b/contracts/test/mocks/MockGateway.sol @@ -116,6 +116,11 @@ contract MockGateway is Gateway { HandlersV2.unlockNativeToken(executor, data); } + // Expose Functions.ensureNotAssetHubAgent for direct unit/fuzz testing. + function exposed_ensureNotAssetHubAgent(bytes32 agentID) external view returns (address) { + return Functions.ensureNotAssetHubAgent(agentID); + } + // Expose internal helper for testing function exposed_v1_transactionBaseGas() external pure returns (uint256) { return v1_transactionBaseGas();