diff --git a/contracts/kernel/Kernel.sol b/contracts/kernel/Kernel.sol index bcbd47436..1ca7d3581 100644 --- a/contracts/kernel/Kernel.sol +++ b/contracts/kernel/Kernel.sol @@ -18,12 +18,16 @@ import "../lib/misc/ERCProxy.sol"; contract Kernel is IKernel, KernelStorage, KernelAppIds, KernelNamespaceConstants, Petrifiable, IsContract, VaultRecoverable, AppProxyFactory, ACLSyntaxSugar { /* Hardcoded constants to save gas bytes32 public constant APP_MANAGER_ROLE = keccak256("APP_MANAGER_ROLE"); + bytes32 public constant APP_MANAGER_EMERGENCY_ROLE = keccak256("APP_MANAGER_EMERGENCY_ROLE"); */ bytes32 public constant APP_MANAGER_ROLE = 0xb6d92708f3d4817afc106147d969e229ced5c46e65e0a5002a0d391287762bd0; + bytes32 public constant APP_MANAGER_EMERGENCY_ROLE = 0xabe3d63c29e614f79a410086a986fb228d44252d5fd75e80688f4621afcf212a; string private constant ERROR_AUTH_FAILED = "KERNEL_AUTH_FAILED"; string private constant ERROR_APP_NOT_CONTRACT = "KERNEL_APP_NOT_CONTRACT"; string private constant ERROR_INVALID_APP_CHANGE = "KERNEL_INVALID_APP_CHANGE"; + string private constant ERROR_MISSING_KILL_SWITCH = "KERNEL_MISSING_KILL_SWITCH"; + string private constant ERROR_BAD_EMERGENCY_UPDATE = "KERNEL_BAD_EMERGENCY_UPDATE"; /** * @dev Constructor that allows the deployer to choose if the base instance should be petrified immediately. @@ -152,6 +156,33 @@ contract Kernel is IKernel, KernelStorage, KernelAppIds, KernelNamespaceConstant _setApp(_namespace, _appId, _app); } + /** + * @notice Set the resolving address of `_appId` in namespace `_namespace` to `_newBaseApp` + * @dev Set the resolving address of a base implementation on emergency. This is entry point works as a particular + * case of `setApp(bytes32,bytes32,address)` that is meant to be used only when the base app to be updated is + * compromised based on the kill switch of the current kernel. This function will perform the proposed update + * only when the current base app address is disabled by the kill-switch and the new proposed one is not. + * Note that this function is not expected to be used for the app addresses namespace, since the kill switch is + * supposed to work with a whitelist of app instances but a blacklist of base implementations. + * @param _namespace App namespace to use + * @param _appId Identifier for app + * @param _newBaseApp Address of the new proposed base app implementation + */ + function setAppOnEmergency(bytes32 _namespace, bytes32 _appId, address _newBaseApp) + public + auth(APP_MANAGER_EMERGENCY_ROLE, arr(_namespace, _appId)) + { + IKillSwitch _killSwitch = killSwitch(); + require(address(_killSwitch) != address(0), ERROR_MISSING_KILL_SWITCH); + + address _currentBaseApp = _getApp(_namespace, _appId); + bool _isCurrentBaseAppEnabled = _killSwitch.shouldDenyCallingBaseApp(_appId, _currentBaseApp); + bool _isNewBaseAppEnabled = _killSwitch.shouldDenyCallingBaseApp(_appId, _newBaseApp); + require(_isCurrentBaseAppEnabled && !_isNewBaseAppEnabled, ERROR_BAD_EMERGENCY_UPDATE); + + _setApp(_namespace, _appId, _newBaseApp); + } + /** * @dev Set the default vault id for the escape hatch mechanism * @param _recoveryVaultAppId Identifier of the recovery vault app @@ -167,6 +198,7 @@ contract Kernel is IKernel, KernelStorage, KernelAppIds, KernelNamespaceConstant * @dev Tells whether a call to an instance of an app should be denied or not based on the kill-switch settings. * Initialization check is implicitly provided by the KillSwitch's existence, as apps can only be installed after initialization. * @param _appId Identifier for app to be checked + * @param _instance Address of the app instance to be checked * @return True if the given call should be denied, false otherwise */ function isAppDisabled(bytes32 _appId, address _instance) public view returns (bool) { @@ -196,7 +228,7 @@ contract Kernel is IKernel, KernelStorage, KernelAppIds, KernelNamespaceConstant * @return Address of the app */ function getApp(bytes32 _namespace, bytes32 _appId) public view returns (address) { - return apps[_namespace][_appId]; + return _getApp(_namespace, _appId); } /** @@ -204,23 +236,23 @@ contract Kernel is IKernel, KernelStorage, KernelAppIds, KernelNamespaceConstant * @return Address of the Vault */ function getRecoveryVault() public view returns (address) { - return apps[KERNEL_APP_ADDR_NAMESPACE][recoveryVaultAppId]; + return _getInstalledApp(recoveryVaultAppId); } /** - * @dev Get the default ACL app + * @dev Get the default installed ACL app * @return ACL app */ function acl() public view returns (IACL) { - return IACL(getApp(KERNEL_APP_ADDR_NAMESPACE, KERNEL_DEFAULT_ACL_APP_ID)); + return IACL(_getInstalledApp(KERNEL_DEFAULT_ACL_APP_ID)); } /** - * @dev Get the default KillSwitch app + * @dev Get the default installed KillSwitch app * @return KillSwitch app */ function killSwitch() public view returns (IKillSwitch) { - return IKillSwitch(getApp(KERNEL_APP_ADDR_NAMESPACE, KERNEL_DEFAULT_KILL_SWITCH_APP_ID)); + return IKillSwitch(_getInstalledApp(KERNEL_DEFAULT_KILL_SWITCH_APP_ID)); } /** @@ -238,6 +270,18 @@ contract Kernel is IKernel, KernelStorage, KernelAppIds, KernelNamespaceConstant defaultAcl.hasPermission(_who, _where, _what, _how); } + function _getBaseApp(bytes32 _appId) internal view returns (address) { + return _getApp(KERNEL_APP_BASES_NAMESPACE, _appId); + } + + function _getInstalledApp(bytes32 _appId) internal view returns (address) { + return _getApp(KERNEL_APP_ADDR_NAMESPACE, _appId); + } + + function _getApp(bytes32 _namespace, bytes32 _appId) internal view returns (address) { + return apps[_namespace][_appId]; + } + function _setApp(bytes32 _namespace, bytes32 _appId, address _app) internal { require(isContract(_app), ERROR_APP_NOT_CONTRACT); apps[_namespace][_appId] = _app; diff --git a/contracts/kill-switch/IKillSwitch.sol b/contracts/kill-switch/IKillSwitch.sol index cacec5ff8..4c8948945 100644 --- a/contracts/kill-switch/IKillSwitch.sol +++ b/contracts/kill-switch/IKillSwitch.sol @@ -4,5 +4,6 @@ import "./IIssuesRegistry.sol"; contract IKillSwitch { - function shouldDenyCallingApp(bytes32 _appId, address _base, address _proxy) external view returns (bool); + function shouldDenyCallingApp(bytes32 _appId, address _base, address _instance) external view returns (bool); + function shouldDenyCallingBaseApp(bytes32 _appId, address _base) public view returns (bool); } diff --git a/contracts/kill-switch/KillSwitch.sol b/contracts/kill-switch/KillSwitch.sol index 6e6a3b21a..c1e917f1b 100644 --- a/contracts/kill-switch/KillSwitch.sol +++ b/contracts/kill-switch/KillSwitch.sol @@ -91,17 +91,22 @@ contract KillSwitch is IKillSwitch, IsContract, AragonApp { * and we have several tests to make sure its usage is working as expected. */ function shouldDenyCallingApp(bytes32 _appId, address _base, address _instance) external view returns (bool) { - // if the instance is the kill switch itself, then allow given call + // If the instance is the kill switch itself, then allow given call if (_instance == address(this)) { return false; } - // if the instance is whitelisted, then allow given call + // If the instance is whitelisted, then allow given call if (isInstanceWhitelisted(_instance)) { return false; } - // if the base implementation is blacklisted, then deny given call + // Check if the base implementation is allowed + return shouldDenyCallingBaseApp(_appId, _base); + } + + function shouldDenyCallingBaseApp(bytes32 _appId, address _base) public view returns (bool) { + // If the base implementation is blacklisted, then deny given call if (isBaseImplementationBlacklisted(_base)) { return true; } diff --git a/contracts/test/mocks/common/KeccakConstants.sol b/contracts/test/mocks/common/KeccakConstants.sol index 6ccd3adc6..fb376f401 100644 --- a/contracts/test/mocks/common/KeccakConstants.sol +++ b/contracts/test/mocks/common/KeccakConstants.sol @@ -18,6 +18,7 @@ contract KeccakConstants { bytes32 public constant KERNEL_APP_ADDR_NAMESPACE = keccak256(abi.encodePacked("app")); bytes32 public constant APP_MANAGER_ROLE = keccak256(abi.encodePacked("APP_MANAGER_ROLE")); + bytes32 public constant APP_MANAGER_EMERGENCY_ROLE = keccak256(abi.encodePacked("APP_MANAGER_EMERGENCY_ROLE")); bytes32 public constant KERNEL_APP_ID = keccak256(abi.encodePacked(APM_NODE, keccak256("kernel"))); bytes32 public constant DEFAULT_ACL_APP_ID = keccak256(abi.encodePacked(APM_NODE, keccak256("acl"))); diff --git a/contracts/test/mocks/kill-switch/EmergencyAppManagerMock.sol b/contracts/test/mocks/kill-switch/EmergencyAppManagerMock.sol new file mode 100644 index 000000000..f512c19cb --- /dev/null +++ b/contracts/test/mocks/kill-switch/EmergencyAppManagerMock.sol @@ -0,0 +1,38 @@ +pragma solidity 0.4.24; + +import "../../../kernel/Kernel.sol"; +import "../../../apps/AragonApp.sol"; + + +/** +* @title EmergencyAppManagerMock +* @dev This mock mimics a contract that is allowed to manage contract upgrades in emergency situations +*/ +contract EmergencyAppManagerMock is AragonApp { + bytes32 public constant APP_MANAGER_EMERGENCY_ROLE = keccak256("APP_MANAGER_EMERGENCY_ROLE"); + + function initialize() external onlyInit { + initialized(); + } + + function setAppOnEmergency(bytes32 _namespace, bytes32 _appId, address _app) public auth(APP_MANAGER_EMERGENCY_ROLE) { + Kernel(kernel()).setAppOnEmergency(_namespace, _appId, _app); + } +} + + +/** +* @title AppManagerMock +* @dev This mock mimics a contract that is allowed to manage contract upgrades in non-emergency situations +*/ +contract AppManagerMock is AragonApp { + bytes32 public constant APP_MANAGER_ROLE = keccak256("APP_MANAGER_ROLE"); + + function initialize() external onlyInit { + initialized(); + } + + function setApp(bytes32 _namespace, bytes32 _appId, address _app) public auth(APP_MANAGER_ROLE) { + kernel().setApp(_namespace, _appId, _app); + } +} diff --git a/contracts/test/mocks/kill-switch/RevertingKillSwitchMock.sol b/contracts/test/mocks/kill-switch/RevertingKillSwitchMock.sol index c2f5f003c..04bf3a15f 100644 --- a/contracts/test/mocks/kill-switch/RevertingKillSwitchMock.sol +++ b/contracts/test/mocks/kill-switch/RevertingKillSwitchMock.sol @@ -9,4 +9,8 @@ contract RevertingKillSwitchMock is KillSwitch { function shouldDenyCallingApp(bytes32 _appId, address _base, address _instance) external view returns (bool) { revert(ERROR_MESSAGE); } + + function shouldDenyCallingBaseApp(bytes32 _appId, address _base) public view returns (bool) { + revert(ERROR_MESSAGE); + } } diff --git a/test/contracts/common/keccak_constants.js b/test/contracts/common/keccak_constants.js index 8d460495b..89c1fa98b 100644 --- a/test/contracts/common/keccak_constants.js +++ b/test/contracts/common/keccak_constants.js @@ -34,6 +34,7 @@ contract('Constants', () => { const kernel = await getContract('Kernel').new(false) assert.equal(await kernel.APP_MANAGER_ROLE(), await keccakConstants.APP_MANAGER_ROLE(), "app manager role doesn't match") + assert.equal(await kernel.APP_MANAGER_EMERGENCY_ROLE(), await keccakConstants.APP_MANAGER_EMERGENCY_ROLE(), "app manager emergency role doesn't match") assert.equal(await kernel.KERNEL_APP_ID(), await keccakConstants.KERNEL_APP_ID(), "app id doesn't match") assert.equal(await kernel.DEFAULT_ACL_APP_ID(), await keccakConstants.DEFAULT_ACL_APP_ID(), "default acl id doesn't match") assert.equal(await kernel.DEFAULT_KILL_SWITCH_APP_ID(), await keccakConstants.DEFAULT_KILL_SWITCH_APP_ID(), "default kill switch id doesn't match") diff --git a/test/contracts/kernel/kernel_lifecycle.js b/test/contracts/kernel/kernel_lifecycle.js index 5425b38bc..832a05295 100644 --- a/test/contracts/kernel/kernel_lifecycle.js +++ b/test/contracts/kernel/kernel_lifecycle.js @@ -23,7 +23,7 @@ contract('Kernel lifecycle', ([root, someone]) => { assert.isFalse(await kernel.hasPermission(someone, kernel.address, APP_MANAGER_ROLE, EMPTY_BYTES)) await assertRevert(kernel.newAppInstance(APP_ID, appBase.address, EMPTY_BYTES, false)) - await assertRevert(kernel.newPinnedAppInstance(APP_ID, appBase.address, EMPTY_BYTES, false)) + await assertRevert(kernel.newPinnedAppInstance(APP_ID, appBase.address)) await assertRevert(kernel.setApp(APP_BASES_NAMESPACE, APP_ID, appBase.address)) await assertRevert(kernel.setRecoveryVaultAppId(VAULT_ID)) } diff --git a/test/contracts/kill-switch/kill_switch_kernel.js b/test/contracts/kill-switch/kill_switch_kernel.js index 6445288a1..f0b526bdd 100644 --- a/test/contracts/kill-switch/kill_switch_kernel.js +++ b/test/contracts/kill-switch/kill_switch_kernel.js @@ -10,6 +10,8 @@ const IssuesRegistry = artifacts.require('IssuesRegistry') const KillSwitchedApp = artifacts.require('KillSwitchedAppMock') const EVMScriptRegistryFactory = artifacts.require('EVMScriptRegistryFactory') +const AppManagerMock = artifacts.require('AppManagerMock') +const EmergencyAppManagerMock = artifacts.require('EmergencyAppManagerMock') const RevertingKillSwitchMock = artifacts.require('RevertingKillSwitchMock') const KernelWithoutKillSwitchMock = artifacts.require('KernelWithoutKillSwitchMock') const KernelWithNonCompliantKillSwitchMock = artifacts.require('KernelWithNonCompliantKillSwitchMock') @@ -21,7 +23,8 @@ contract('KillSwitch Kernel', ([_, root, owner, securityPartner]) => { let dao, acl, app, registryFactory let kernelBase, aclBase, appBase, killSwitchBase, issuesRegistryBase, daoFactory let kernelWithoutKillSwitchBase, kernelWithNonCompliantKillSwitchBase, failingKillSwitchBase - let CORE_NAMESPACE, KERNEL_APP_ID, APP_MANAGER_ROLE, CHANGE_SEVERITY_ROLE, CHANGE_DEFAULT_ISSUES_REGISTRY_ROLE, CHANGE_ISSUES_REGISTRY_ROLE, CHANGE_WHITELISTED_INSTANCES_ROLE, CHANGE_BLACKLISTED_BASE_IMPLS_ROLE, CHANGE_HIGHEST_ALLOWED_SEVERITY_ROLE, WRITER_ROLE + let CORE_NAMESPACE, APP_ADDR_NAMESPACE, APP_BASES_NAMESPACE, KERNEL_APP_ID, APP_MANAGER_ROLE, APP_MANAGER_EMERGENCY_ROLE, WRITER_ROLE + let CHANGE_SEVERITY_ROLE, CHANGE_DEFAULT_ISSUES_REGISTRY_ROLE, CHANGE_ISSUES_REGISTRY_ROLE, CHANGE_WHITELISTED_INSTANCES_ROLE, CHANGE_BLACKLISTED_BASE_IMPLS_ROLE, CHANGE_HIGHEST_ALLOWED_SEVERITY_ROLE before('deploy base implementations', async () => { // real @@ -39,10 +42,14 @@ contract('KillSwitch Kernel', ([_, root, owner, securityPartner]) => { }) before('load constants and roles', async () => { - WRITER_ROLE = await appBase.WRITER_ROLE() CORE_NAMESPACE = await kernelBase.CORE_NAMESPACE() + APP_ADDR_NAMESPACE = await kernelBase.APP_ADDR_NAMESPACE() + APP_BASES_NAMESPACE = await kernelBase.APP_BASES_NAMESPACE() KERNEL_APP_ID = await kernelBase.KERNEL_APP_ID() + + WRITER_ROLE = await appBase.WRITER_ROLE() APP_MANAGER_ROLE = await kernelBase.APP_MANAGER_ROLE() + APP_MANAGER_EMERGENCY_ROLE = await kernelBase.APP_MANAGER_EMERGENCY_ROLE() CHANGE_SEVERITY_ROLE = await issuesRegistryBase.CHANGE_SEVERITY_ROLE() CHANGE_DEFAULT_ISSUES_REGISTRY_ROLE = await killSwitchBase.CHANGE_DEFAULT_ISSUES_REGISTRY_ROLE() CHANGE_ISSUES_REGISTRY_ROLE = await killSwitchBase.CHANGE_ISSUES_REGISTRY_ROLE() @@ -315,257 +322,409 @@ contract('KillSwitch Kernel', ([_, root, owner, securityPartner]) => { beforeEach('create kill switched app', async () => { const initializeData = appBase.contract.initialize.getData(owner) - const receipt = await dao.newAppInstance(SAMPLE_APP_ID, appBase.address, initializeData, false, { from: root }) + const receipt = await dao.newAppInstance(SAMPLE_APP_ID, appBase.address, initializeData, true, { from: root }) app = KillSwitchedApp.at(getNewProxyAddress(receipt)) await acl.createPermission(owner, app.address, WRITER_ROLE, root, { from: root }) }) - context('when the function being called is not tagged', () => { + describe('isAppEnabled', () => { + context('when the function being called is not tagged', () => { - const itExecutesTheCallEvenWhenBaseImplementationIsBlacklisted = () => { - const itExecutesTheCall = () => { - it('executes the call', async () => { - assert.equal(await app.read(), 42) - }) - } - - context('when the instance being called is whitelisted', () => { - beforeEach('whitelist instance', async () => { - await killSwitch.setWhitelistedInstance(app.address, true, { from: owner }) - }) + const itExecutesTheCallEvenWhenBaseImplementationIsBlacklisted = () => { + const itExecutesTheCall = () => { + it('executes the call', async () => { + assert.equal(await app.read(), 42) + }) + } - context('when the base implementation is not blacklisted', () => { - beforeEach('do not blacklist base implementation', async () => { - await killSwitch.setBlacklistedBaseImplementation(appBase.address, false, { from: owner }) + context('when the instance being called is whitelisted', () => { + beforeEach('whitelist instance', async () => { + await killSwitch.setWhitelistedInstance(app.address, true, { from: owner }) }) - itExecutesTheCall() - }) + context('when the base implementation is not blacklisted', () => { + beforeEach('do not blacklist base implementation', async () => { + await killSwitch.setBlacklistedBaseImplementation(appBase.address, false, { from: owner }) + }) - context('when the base implementation is blacklisted', () => { - beforeEach('blacklist base implementation', async () => { - await killSwitch.setBlacklistedBaseImplementation(appBase.address, true, { from: owner }) + itExecutesTheCall() }) - itExecutesTheCall() - }) - }) + context('when the base implementation is blacklisted', () => { + beforeEach('blacklist base implementation', async () => { + await killSwitch.setBlacklistedBaseImplementation(appBase.address, true, { from: owner }) + }) - context('when the instance being called is not marked as whitelisted', () => { - beforeEach('dot not whitelist instance', async () => { - await killSwitch.setWhitelistedInstance(app.address, false, { from: owner }) + itExecutesTheCall() + }) }) - context('when the base implementation is not blacklisted', () => { - beforeEach('do not blacklist base implementation', async () => { - await killSwitch.setBlacklistedBaseImplementation(appBase.address, false, { from: owner }) + context('when the instance being called is not marked as whitelisted', () => { + beforeEach('dot not whitelist instance', async () => { + await killSwitch.setWhitelistedInstance(app.address, false, { from: owner }) }) - itExecutesTheCall() - }) + context('when the base implementation is not blacklisted', () => { + beforeEach('do not blacklist base implementation', async () => { + await killSwitch.setBlacklistedBaseImplementation(appBase.address, false, { from: owner }) + }) - context('when the base implementation is blacklisted', () => { - beforeEach('blacklist base implementation', async () => { - await killSwitch.setBlacklistedBaseImplementation(appBase.address, true, { from: owner }) + itExecutesTheCall() }) - itExecutesTheCall() - }) - }) - } + context('when the base implementation is blacklisted', () => { + beforeEach('blacklist base implementation', async () => { + await killSwitch.setBlacklistedBaseImplementation(appBase.address, true, { from: owner }) + }) - context('when there is no bug registered', () => { - itExecutesTheCallEvenWhenBaseImplementationIsBlacklisted() - }) + itExecutesTheCall() + }) + }) + } - context('when there is a bug registered', () => { - beforeEach('register a bug', async () => { - await defaultIssuesRegistry.setSeverityFor(appBase.address, SEVERITY.MID, { from: securityPartner }) + context('when there is no bug registered', () => { + itExecutesTheCallEvenWhenBaseImplementationIsBlacklisted() }) - itExecutesTheCallEvenWhenBaseImplementationIsBlacklisted() - }) - }) + context('when there is a bug registered', () => { + beforeEach('register a bug', async () => { + await defaultIssuesRegistry.setSeverityFor(appBase.address, SEVERITY.MID, { from: securityPartner }) + }) - context('when the function being called is tagged', () => { - const itExecutesTheCall = () => { - it('executes the call', async () => { - await app.write(10, { from: owner }) - assert.equal(await app.read(), 10) + itExecutesTheCallEvenWhenBaseImplementationIsBlacklisted() }) - } + }) - const itDoesNotExecuteTheCall = () => { - it('does not execute the call', async () => { - await assertRevert(app.write(10, { from: owner }), 'APP_AUTH_FAILED') - }) - } + context('when the function being called is tagged', () => { + const itExecutesTheCall = () => { + it('executes the call', async () => { + await app.write(10, { from: owner }) + assert.equal(await app.read(), 10) + }) + } - const itExecutesTheCallOnlyWhenWhitelisted = () => { - context('when the instance being called is whitelisted', () => { - beforeEach('whitelist instance', async () => { - await killSwitch.setWhitelistedInstance(app.address, true, { from: owner }) + const itDoesNotExecuteTheCall = () => { + it('does not execute the call', async () => { + await assertRevert(app.write(10, { from: owner }), 'APP_AUTH_FAILED') }) + } - context('when the base implementation is not blacklisted', () => { - beforeEach('do not blacklist base implementation', async () => { - await killSwitch.setBlacklistedBaseImplementation(appBase.address, false, { from: owner }) + const itExecutesTheCallOnlyWhenWhitelisted = () => { + context('when the instance being called is whitelisted', () => { + beforeEach('whitelist instance', async () => { + await killSwitch.setWhitelistedInstance(app.address, true, { from: owner }) }) - itExecutesTheCall() - }) + context('when the base implementation is not blacklisted', () => { + beforeEach('do not blacklist base implementation', async () => { + await killSwitch.setBlacklistedBaseImplementation(appBase.address, false, { from: owner }) + }) - context('when the base implementation is blacklisted', () => { - beforeEach('blacklist base implementation', async () => { - await killSwitch.setBlacklistedBaseImplementation(appBase.address, true, { from: owner }) + itExecutesTheCall() }) - itExecutesTheCall() - }) - }) + context('when the base implementation is blacklisted', () => { + beforeEach('blacklist base implementation', async () => { + await killSwitch.setBlacklistedBaseImplementation(appBase.address, true, { from: owner }) + }) - context('when the instance being called is not marked as whitelisted', () => { - beforeEach('dot not whitelist instance', async () => { - await killSwitch.setWhitelistedInstance(app.address, false, { from: owner }) + itExecutesTheCall() + }) }) - context('when the base implementation is not blacklisted', () => { - beforeEach('do not blacklist base implementation', async () => { - await killSwitch.setBlacklistedBaseImplementation(appBase.address, false, { from: owner }) + context('when the instance being called is not marked as whitelisted', () => { + beforeEach('dot not whitelist instance', async () => { + await killSwitch.setWhitelistedInstance(app.address, false, { from: owner }) }) - itDoesNotExecuteTheCall() - }) + context('when the base implementation is not blacklisted', () => { + beforeEach('do not blacklist base implementation', async () => { + await killSwitch.setBlacklistedBaseImplementation(appBase.address, false, { from: owner }) + }) - context('when the base implementation is blacklisted', () => { - beforeEach('blacklist base implementation', async () => { - await killSwitch.setBlacklistedBaseImplementation(appBase.address, true, { from: owner }) + itDoesNotExecuteTheCall() }) - itDoesNotExecuteTheCall() - }) - }) - } + context('when the base implementation is blacklisted', () => { + beforeEach('blacklist base implementation', async () => { + await killSwitch.setBlacklistedBaseImplementation(appBase.address, true, { from: owner }) + }) - const itExecutesTheCallUnlessInstanceNotWhitelistedAndBaseBlacklisted = () => { - context('when the instance being called is whitelisted', () => { - beforeEach('whitelist instance', async () => { - await killSwitch.setWhitelistedInstance(app.address, true, { from: owner }) + itDoesNotExecuteTheCall() + }) }) + } - context('when the base implementation is not blacklisted', () => { - beforeEach('do not blacklist base implementation', async () => { - await killSwitch.setBlacklistedBaseImplementation(appBase.address, false, { from: owner }) + const itExecutesTheCallUnlessInstanceNotWhitelistedAndBaseBlacklisted = () => { + context('when the instance being called is whitelisted', () => { + beforeEach('whitelist instance', async () => { + await killSwitch.setWhitelistedInstance(app.address, true, { from: owner }) }) - itExecutesTheCall() + context('when the base implementation is not blacklisted', () => { + beforeEach('do not blacklist base implementation', async () => { + await killSwitch.setBlacklistedBaseImplementation(appBase.address, false, { from: owner }) + }) + + itExecutesTheCall() + }) + + context('when the base implementation is blacklisted', () => { + beforeEach('blacklist base implementation', async () => { + await killSwitch.setBlacklistedBaseImplementation(appBase.address, true, { from: owner }) + }) + + itExecutesTheCall() + }) }) - context('when the base implementation is blacklisted', () => { - beforeEach('blacklist base implementation', async () => { - await killSwitch.setBlacklistedBaseImplementation(appBase.address, true, { from: owner }) + context('when the instance being called is not marked as whitelisted', () => { + beforeEach('dot not whitelist instance', async () => { + await killSwitch.setWhitelistedInstance(app.address, false, { from: owner }) }) - itExecutesTheCall() + context('when the base implementation is not blacklisted', () => { + beforeEach('do not blacklist base implementation', async () => { + await killSwitch.setBlacklistedBaseImplementation(appBase.address, false, { from: owner }) + }) + + itExecutesTheCall() + }) + + context('when the base implementation is blacklisted', () => { + beforeEach('blacklist base implementation', async () => { + await killSwitch.setBlacklistedBaseImplementation(appBase.address, true, { from: owner }) + }) + + itDoesNotExecuteTheCall() + }) }) + } + + context('when there is no bug registered', () => { + itExecutesTheCallUnlessInstanceNotWhitelistedAndBaseBlacklisted() }) - context('when the instance being called is not marked as whitelisted', () => { - beforeEach('dot not whitelist instance', async () => { - await killSwitch.setWhitelistedInstance(app.address, false, { from: owner }) + context('when there is a bug registered', () => { + beforeEach('register a bug', async () => { + await defaultIssuesRegistry.setSeverityFor(appBase.address, SEVERITY.MID, { from: securityPartner }) }) - context('when the base implementation is not blacklisted', () => { - beforeEach('do not blacklist base implementation', async () => { - await killSwitch.setBlacklistedBaseImplementation(appBase.address, false, { from: owner }) + context('when the bug was real', () => { + context('when there is no highest allowed severity set for the contract being called', () => { + itExecutesTheCallOnlyWhenWhitelisted() }) - itExecutesTheCall() + context('when there is a highest allowed severity set for the contract being called', () => { + context('when the highest allowed severity is under the reported bug severity', () => { + beforeEach('set highest allowed severity below the one reported', async () => { + await killSwitch.setHighestAllowedSeverity(SAMPLE_APP_ID, SEVERITY.LOW, { from: owner }) + }) + + itExecutesTheCallOnlyWhenWhitelisted() + }) + + context('when the highest allowed severity is equal to the reported bug severity', () => { + beforeEach('set highest allowed severity equal to the one reported', async () => { + await killSwitch.setHighestAllowedSeverity(SAMPLE_APP_ID, SEVERITY.MID, { from: owner }) + }) + + itExecutesTheCallUnlessInstanceNotWhitelistedAndBaseBlacklisted() + }) + + context('when the highest allowed severity is greater than the reported bug severity', () => { + beforeEach('set highest allowed severity above the one reported', async () => { + await killSwitch.setHighestAllowedSeverity(SAMPLE_APP_ID, SEVERITY.CRITICAL, { from: owner }) + }) + + itExecutesTheCallUnlessInstanceNotWhitelistedAndBaseBlacklisted() + }) + }) }) - context('when the base implementation is blacklisted', () => { - beforeEach('blacklist base implementation', async () => { - await killSwitch.setBlacklistedBaseImplementation(appBase.address, true, { from: owner }) + context('when the bug was a false positive', () => { + beforeEach('roll back reported bug', async () => { + await defaultIssuesRegistry.setSeverityFor(appBase.address, SEVERITY.NONE, { from: securityPartner }) + }) + + context('when there is no highest allowed severity set for the contract being called', () => { + itExecutesTheCallUnlessInstanceNotWhitelistedAndBaseBlacklisted() }) - itDoesNotExecuteTheCall() + context('when there is a highest allowed severity set for the contract being called', () => { + context('when the highest allowed severity is under the reported bug severity', () => { + beforeEach('set highest allowed severity below the one reported', async () => { + await killSwitch.setHighestAllowedSeverity(SAMPLE_APP_ID, SEVERITY.LOW, { from: owner }) + }) + + itExecutesTheCallUnlessInstanceNotWhitelistedAndBaseBlacklisted() + }) + + context('when the highest allowed severity is equal to the reported bug severity', () => { + beforeEach('set highest allowed severity equal to the one reported', async () => { + await killSwitch.setHighestAllowedSeverity(SAMPLE_APP_ID, SEVERITY.MID, { from: owner }) + }) + + itExecutesTheCallUnlessInstanceNotWhitelistedAndBaseBlacklisted() + }) + + context('when the highest allowed severity is greater than the reported bug severity', () => { + beforeEach('set highest allowed severity above the one reported', async () => { + await killSwitch.setHighestAllowedSeverity(SAMPLE_APP_ID, SEVERITY.CRITICAL, { from: owner }) + }) + + itExecutesTheCallUnlessInstanceNotWhitelistedAndBaseBlacklisted() + }) + }) }) }) - } + }) + }) - context('when there is no bug registered', () => { - itExecutesTheCallUnlessInstanceNotWhitelistedAndBaseBlacklisted() + describe('setAppOnEmergency', () => { + const MANAGER_APP_ID = '0x22222' + const EMERGENCY_MANAGER_APP_ID = '0x33333' + + let appBaseV2, managerAppBaseV1, managerAppBaseV2, emergencyManagerAppBase, managerApp, emergencyManagerApp + + before('deploy base implementations', async () => { + appBaseV2 = await KillSwitchedApp.new() + managerAppBaseV1 = await AppManagerMock.new() + managerAppBaseV2 = await AppManagerMock.new() + emergencyManagerAppBase = await EmergencyAppManagerMock.new() }) - context('when there is a bug registered', () => { - beforeEach('register a bug', async () => { - await defaultIssuesRegistry.setSeverityFor(appBase.address, SEVERITY.MID, { from: securityPartner }) + beforeEach('create emergency manager app', async () => { + // create emergency manager app + const emergencyManagerReceipt = await dao.newAppInstance(EMERGENCY_MANAGER_APP_ID, emergencyManagerAppBase.address, '0x', true, {from: root}) + emergencyManagerApp = EmergencyAppManagerMock.at(getNewProxyAddress(emergencyManagerReceipt)) + await emergencyManagerApp.initialize() + + // grant APP_MANAGER_EMERGENCY_ROLE permission + await acl.createPermission(emergencyManagerApp.address, dao.address, APP_MANAGER_EMERGENCY_ROLE, root, { from: root }) + await acl.createPermission(root, emergencyManagerApp.address, APP_MANAGER_EMERGENCY_ROLE, root, { from: root }) + }) + + beforeEach('create manager app', async () => { + // create manager app instance and whitelist it in the kill-switch + const managerReceipt = await dao.newAppInstance(MANAGER_APP_ID, managerAppBaseV1.address, '0x', true, { from: root }) + managerApp = AppManagerMock.at(getNewProxyAddress(managerReceipt)) + await managerApp.initialize() + + // set manager app as the APP_MANAGER_ROLE permissions manager + await acl.createPermission(root, managerApp.address, APP_MANAGER_ROLE, root, { from: root }) + await acl.grantPermission(managerApp.address, dao.address, APP_MANAGER_ROLE, { from: root }) + await acl.revokePermission(root, dao.address, APP_MANAGER_ROLE, { from: root }) + await acl.setPermissionManager(managerApp.address, dao.address, APP_MANAGER_ROLE, { from: root }) + }) + + it('has a manager app installed with root authority permissions', async () => { + assert.equal(await dao.getApp(APP_ADDR_NAMESPACE, MANAGER_APP_ID), managerApp.address, 'manager app instance does not match') + assert.isTrue(await acl.hasPermission(managerApp.address, dao.address, APP_MANAGER_ROLE), 'manager app should have APP_MANAGER_ROLE permissions') + assert.equal(await acl.getPermissionManager(dao.address, APP_MANAGER_ROLE), managerApp.address, 'manager app should be the APP_MANAGER_ROLE permissions manager of the DAO') + }) + + it('has a emergency manager app installed with emergency permissions', async () => { + assert.equal(await dao.getApp(APP_ADDR_NAMESPACE, EMERGENCY_MANAGER_APP_ID), emergencyManagerApp.address, 'emergency manager app instance does not match') + assert.isTrue(await acl.hasPermission(emergencyManagerApp.address, dao.address, APP_MANAGER_EMERGENCY_ROLE), 'emergency manager app should have APP_MANAGER_EMERGENCY_ROLE permissions') + }) + + context('with a non kill-switched contract', () => { + context('when trying to update it through the manager app', () => { + it('can be updated', async () => { + assert.equal(await dao.getApp(APP_BASES_NAMESPACE, SAMPLE_APP_ID), appBase.address, 'sample app should be V1') + assert.isFalse(await killSwitch.shouldDenyCallingApp(SAMPLE_APP_ID, appBase.address, app.address), 'sample app should not be kill-switched') + await app.write(10, { from: owner }) + assert.equal(await app.read(), 10) + + await managerApp.setApp(APP_BASES_NAMESPACE, SAMPLE_APP_ID, appBaseV2.address, { from: root }) + + assert.equal(await dao.getApp(APP_BASES_NAMESPACE, SAMPLE_APP_ID), appBaseV2.address, 'sample app should be V2') + assert.isFalse(await killSwitch.shouldDenyCallingApp(SAMPLE_APP_ID, appBaseV2.address, app.address), 'sample app should not be kill-switched') + await app.write(12, { from: owner }) + assert.equal(await app.read(), 12) + }) }) - context('when the bug was real', () => { - context('when there is no highest allowed severity set for the contract being called', () => { - itExecutesTheCallOnlyWhenWhitelisted() + context('when trying to update it through the emergency manager app', () => { + it('reverts', async () => { + await assertRevert(emergencyManagerApp.setAppOnEmergency(APP_BASES_NAMESPACE, SAMPLE_APP_ID, appBaseV2.address, { from: root }), 'KERNEL_BAD_EMERGENCY_UPDATE') }) + }) + }) - context('when there is a highest allowed severity set for the contract being called', () => { - context('when the highest allowed severity is under the reported bug severity', () => { - beforeEach('set highest allowed severity below the one reported', async () => { - await killSwitch.setHighestAllowedSeverity(SAMPLE_APP_ID, SEVERITY.LOW, { from: owner }) - }) + context('with a kill-switched base app', () => { + beforeEach('kill switch base app', async () => { + assert.isFalse(await killSwitch.shouldDenyCallingApp(SAMPLE_APP_ID, appBase.address, app.address), 'sample app should not be kill-switched') + await app.write(10, { from: owner }) + assert.equal(await app.read(), 10) - itExecutesTheCallOnlyWhenWhitelisted() - }) + await killSwitch.setBlacklistedBaseImplementation(appBase.address, true, { from: owner }) + assert.isTrue(await killSwitch.shouldDenyCallingApp(SAMPLE_APP_ID, appBase.address, app.address), 'sample app should be kill-switched') + await assertRevert(app.write(12, { from: owner }), 'APP_AUTH_FAILED') + }) - context('when the highest allowed severity is equal to the reported bug severity', () => { - beforeEach('set highest allowed severity equal to the one reported', async () => { - await killSwitch.setHighestAllowedSeverity(SAMPLE_APP_ID, SEVERITY.MID, { from: owner }) - }) + context('when the manager app is not kill switched', () => { + context('when trying to update it through the manager app', () => { + it('can be updated', async () => { + await managerApp.setApp(APP_BASES_NAMESPACE, SAMPLE_APP_ID, appBaseV2.address, { from: root }) + assert.equal(await dao.getApp(APP_BASES_NAMESPACE, SAMPLE_APP_ID), appBaseV2.address, 'sample app should have been updated') + assert.isFalse(await killSwitch.shouldDenyCallingApp(SAMPLE_APP_ID, appBaseV2.address, app.address), 'new sample app should not be kill-switched') - itExecutesTheCallUnlessInstanceNotWhitelistedAndBaseBlacklisted() + assert.equal(await app.read(), 10) + await app.write(12, { from: owner }) + assert.equal(await app.read(), 12) }) + }) - context('when the highest allowed severity is greater than the reported bug severity', () => { - beforeEach('set highest allowed severity above the one reported', async () => { - await killSwitch.setHighestAllowedSeverity(SAMPLE_APP_ID, SEVERITY.CRITICAL, { from: owner }) - }) + context('when trying to update it through the emergency manager app', () => { + it('can be updated', async () => { + await emergencyManagerApp.setAppOnEmergency(APP_BASES_NAMESPACE, SAMPLE_APP_ID, appBaseV2.address, { from: root }) + assert.equal(await dao.getApp(APP_BASES_NAMESPACE, SAMPLE_APP_ID), appBaseV2.address, 'sample app should have been updated') + assert.isFalse(await killSwitch.shouldDenyCallingApp(SAMPLE_APP_ID, appBaseV2.address, app.address), 'new sample app should not be kill-switched') - itExecutesTheCallUnlessInstanceNotWhitelistedAndBaseBlacklisted() + assert.equal(await app.read(), 10) + await app.write(12, { from: owner }) + assert.equal(await app.read(), 12) }) }) }) - context('when the bug was a false positive', () => { - beforeEach('roll back reported bug', async () => { - await defaultIssuesRegistry.setSeverityFor(appBase.address, SEVERITY.NONE, { from: securityPartner }) + context('when the manager app is kill switched', () => { + beforeEach('kill switch manager app', async () => { + await killSwitch.setBlacklistedBaseImplementation(managerAppBaseV1.address, true, { from: owner }) + assert.isTrue(await killSwitch.shouldDenyCallingApp(MANAGER_APP_ID, managerAppBaseV1.address, managerApp.address), 'manager app should be kill-switched') }) - context('when there is no highest allowed severity set for the contract being called', () => { - itExecutesTheCallUnlessInstanceNotWhitelistedAndBaseBlacklisted() + context('when trying to update it through the manager app', () => { + it('reverts', async () => { + await assertRevert(managerApp.setApp(APP_BASES_NAMESPACE, SAMPLE_APP_ID, appBaseV2.address, { from: root }), 'APP_AUTH_FAILED') + }) }) - context('when there is a highest allowed severity set for the contract being called', () => { - context('when the highest allowed severity is under the reported bug severity', () => { - beforeEach('set highest allowed severity below the one reported', async () => { - await killSwitch.setHighestAllowedSeverity(SAMPLE_APP_ID, SEVERITY.LOW, { from: owner }) - }) + context('when trying to update it through the emergency manager app', () => { + it('can be updated directly', async () => { + await emergencyManagerApp.setAppOnEmergency(APP_BASES_NAMESPACE, SAMPLE_APP_ID, appBaseV2.address, { from: root }) + assert.equal(await dao.getApp(APP_BASES_NAMESPACE, SAMPLE_APP_ID), appBaseV2.address, 'sample app should have been updated') + assert.isFalse(await killSwitch.shouldDenyCallingApp(SAMPLE_APP_ID, appBaseV2.address, app.address), 'new sample app should not be kill-switched') - itExecutesTheCallUnlessInstanceNotWhitelistedAndBaseBlacklisted() + assert.equal(await app.read(), 10) + await app.write(12, { from: owner }) + assert.equal(await app.read(), 12) }) - context('when the highest allowed severity is equal to the reported bug severity', () => { - beforeEach('set highest allowed severity equal to the one reported', async () => { - await killSwitch.setHighestAllowedSeverity(SAMPLE_APP_ID, SEVERITY.MID, { from: owner }) - }) - - itExecutesTheCallUnlessInstanceNotWhitelistedAndBaseBlacklisted() - }) + it('can be updated passing through the manager app first', async () => { + await emergencyManagerApp.setAppOnEmergency(APP_BASES_NAMESPACE, MANAGER_APP_ID, managerAppBaseV2.address, { from: root }) + assert.equal(await dao.getApp(APP_BASES_NAMESPACE, MANAGER_APP_ID), managerAppBaseV2.address, 'manager app should have been updated') + assert.isFalse(await killSwitch.shouldDenyCallingApp(MANAGER_APP_ID, managerAppBaseV2.address, managerApp.address), 'new manager app should not be kill-switched') - context('when the highest allowed severity is greater than the reported bug severity', () => { - beforeEach('set highest allowed severity above the one reported', async () => { - await killSwitch.setHighestAllowedSeverity(SAMPLE_APP_ID, SEVERITY.CRITICAL, { from: owner }) - }) + await managerApp.setApp(APP_BASES_NAMESPACE, SAMPLE_APP_ID, appBaseV2.address, { from: root }) + assert.equal(await dao.getApp(APP_BASES_NAMESPACE, SAMPLE_APP_ID), appBaseV2.address, 'sample app should have been updated') + assert.isFalse(await killSwitch.shouldDenyCallingApp(SAMPLE_APP_ID, appBaseV2.address, app.address), 'new sample app should not be kill-switched') - itExecutesTheCallUnlessInstanceNotWhitelistedAndBaseBlacklisted() + assert.equal(await app.read(), 10) + await app.write(12, { from: owner }) + assert.equal(await app.read(), 12) }) }) })