Merge pull request #2340 from ethereum/fix_save

Better checks when saving a file

.circleci/config.yml

@@ -107,7 +107,7 @@ jobs:
       - COMMIT_AUTHOR: "Circle CI"
     working_directory: ~/remix-project
 
-    parallelism: 70
+    parallelism: 80
     steps:
       - browser-tools/install-chrome
       - browser-tools/install-chromedriver
@@ -150,7 +150,7 @@ jobs:
       - COMMIT_AUTHOR: "Circle CI"
     working_directory: ~/remix-project
 
-    parallelism: 70
+    parallelism: 80
     steps:
       - browser-tools/install-firefox
       - browser-tools/install-geckodriver
@@ -192,7 +192,7 @@ jobs:
       - COMMIT_AUTHOR_EMAIL: "yann@ethereum.org"
       - COMMIT_AUTHOR: "Circle CI"
     working_directory: ~/remix-project
-    parallelism: 7
+    parallelism: 10
     steps:
       - browser-tools/install-chrome
       - browser-tools/install-chromedriver

apps/remix-ide-e2e/src/tests/plugin_api.ts

@@ -83,10 +83,35 @@ const checkForAcceptAndRemember = async function (browser: NightwatchBrowser) {
     browser.frameParent(() => {
       browser.pause(1000).element('xpath', '//*[@data-id="permissionHandlerRememberUnchecked"]', (visible: any) => {
         if (visible.status && visible.status === -1) {
+
+          browser.pause(1000).element('xpath', '//*[@data-id="PermissionHandler-modal-footer-ok-react"]', (okPresent: any) => {
+            if ((okPresent.status && okPresent.status === -1) || okPresent.value === false) {
               // @ts-ignore
               browser.frame(0, () => { resolve(true) })
             } else {
-          browser.waitForElementVisible('//*[@data-id="permissionHandlerRememberUnchecked"]').click('//*[@data-id="permissionHandlerRememberUnchecked"]').waitForElementVisible('//*[@data-id="PermissionHandler-modal-footer-ok-react"]').click('//*[@data-id="PermissionHandler-modal-footer-ok-react"]', () => {
+              browser
+              .useXpath()
+              .isVisible('//*[@data-id="PermissionHandler-modal-footer-ok-react"]', (okVisible: any) => {
+                if (okVisible.value) {
+                  browser.click('//*[@data-id="PermissionHandler-modal-footer-ok-react"]', () => {
+                    // @ts-ignore
+                    browser.frame(0, () => { resolve(true) })
+                  })
+                } else {
+                  // @ts-ignore
+                  browser.frame(0, () => { resolve(true) })
+                }
+              })
+            }
+          })
+
+
+
+        } else {
+          browser.waitForElementVisible('//*[@data-id="permissionHandlerRememberUnchecked"]')
+          .click('//*[@data-id="permissionHandlerRememberUnchecked"]')
+          .waitForElementVisible('//*[@data-id="PermissionHandler-modal-footer-ok-react"]')
+          .click('//*[@data-id="PermissionHandler-modal-footer-ok-react"]', () => {
             // @ts-ignore
             browser.frame(0, () => { resolve(true) })
           })

apps/remix-ide-e2e/src/tests/terminal.test.ts

@@ -88,6 +88,7 @@ module.exports = {
       .executeScript('remix.execute(\'resolveExternalUrlAndSaveToaPath.js\')')
       .waitForElementContainsText('*[data-id="terminalJournal"]', 'abstract contract ERC20Burnable', 60000)
       .openFile('.deps/github/newFile.sol')
+
   },
 
   'Deploy "Owner" using an ether.js script, listen to event and check event are logged in the terminal #group4': function (browser: NightwatchBrowser) {

apps/remix-ide/src/app/plugins/permission-handler-plugin.tsx

@@ -70,12 +70,12 @@ export class PermissionHandlerPlugin extends Plugin {
      * @param {string} message from the caller plugin to add more details if needed
      * @returns {Promise<boolean>}
      */
-    async askPermission(from: Profile, to: Profile, method: string, message: string) {
+    async askPermission(from: Profile, to: Profile, method: string, message: string, sensitiveCall: boolean) {
         try {
             this.permissions = this._getFromLocal()
             if (!this.permissions[to.name]) this.permissions[to.name] = {}
             if (!this.permissions[to.name][method]) this.permissions[to.name][method] = {}
-            if (!this.permissions[to.name][method][from.name]) return this.openPermission(from, to, method, message)
+            if (!this.permissions[to.name][method][from.name]) return this.openPermission(from, to, method, message, sensitiveCall)
 
             const { allow, hash } = this.permissions[to.name][method][from.name]
             if (!allow) {
@@ -85,20 +85,21 @@ export class PermissionHandlerPlugin extends Plugin {
             }
             return hash === from.hash
                 ? true // Allow
-                : await this.openPermission(from, to, method, message)
+                : await this.openPermission(from, to, method, message, sensitiveCall)
         } catch (err) {
             throw new Error(err)
         }
     }
 
-    async openPermission(from: Profile, to: Profile, method: string, message: string) {
+    async openPermission(from: Profile, to: Profile, method: string, message: string, sensitiveCall: boolean) {
         const remember = this.permissions[to.name][method][from.name]
         const value: PermissionHandlerValue = {
             from,
             to,
             method,
             message,
-            remember
+            remember,
+            sensitiveCall
         }
         const modal: AppModal = {
             id: 'PermissionHandler',

apps/remix-ide/src/app/tabs/web3-provider.js

@@ -21,7 +21,11 @@ export class Web3ProviderModule extends Plugin {
     Should be taken carefully and probably not be release as it is now.
   */
   sendAsync(payload) {
+
     return new Promise((resolve, reject) => {
+      this.askUserPermission('sendAsync', `Calling ${payload.method} with parameters ${JSON.stringify(payload.params, null, '\t')}`).then(
+        async (result) => {
+          if (result) {
             const provider = this.blockchain.web3().currentProvider
             // see https://github.com/ethereum/web3.js/pull/1018/files#diff-d25786686c1053b786cc2626dc6e048675050593c0ebaafbf0814e1996f22022R129
             provider[provider.sendAsync ? 'sendAsync' : 'send'](payload, async (error, message) => {
@@ -41,6 +45,12 @@ export class Web3ProviderModule extends Plugin {
               }
               resolve(message)
             })
+          } else {
+            reject(new Error('User denied permission'))
+          }
+        }).catch((e) => {
+          reject(e)
+        })
     })
   }
 

apps/remix-ide/src/remixAppManager.js

@@ -12,6 +12,12 @@ const requiredModules = [ // services + layout views + system views
 
 const dependentModules = ['git', 'hardhat', 'truffle', 'slither'] // module which shouldn't be manually activated (e.g git is activated by remixd)
 
+const sensitiveCalls = {
+  'fileManager': ['writeFile', 'copyFile', 'rename', 'copyDir'],
+  'contentImport': ['resolveAndSave'],
+  'web3Provider': ['sendAsync'],
+}
+
 export function isNative(name) {
   const nativePlugins = ['vyper', 'workshops', 'debugger', 'remixd', 'menuicons', 'solidity', 'hardhat-provider', 'solidityStaticAnalysis', 'solidityUnitTesting', 'layout', 'notification', 'hardhat-provider', 'ganache-provider']
   return nativePlugins.includes(name) || requiredModules.includes(name)
@@ -65,6 +71,7 @@ export class RemixAppManager extends PluginManager {
   }
 
   async canCall(from, to, method, message) {
+    const isSensitiveCall = sensitiveCalls[to] && sensitiveCalls[to].includes(method)
     // Make sure the caller of this methods is the target plugin
     if (to !== this.currentRequest.from) {
       return false
@@ -73,8 +80,9 @@ export class RemixAppManager extends PluginManager {
     if (isNative(from)) {
       return true
     }
+
     // ask the user for permission
-    return await this.call('permissionhandler', 'askPermission', this.profiles[from], this.profiles[to], method, message)
+    return await this.call('permissionhandler', 'askPermission', this.profiles[from], this.profiles[to], method, message, isSensitiveCall)
   }
 
   onPluginActivated(plugin) {

libs/remix-core-plugin/src/lib/compiler-content-imports.ts

@@ -124,6 +124,10 @@ export class CompilerImports extends Plugin {
     */
   async resolveAndSave (url, targetPath) {
     try {
+      if (targetPath && this.currentRequest) {
+        const canCall = await this.askUserPermission('resolveAndSave', 'This action will update the path ' + targetPath)
+        if (!canCall) throw new Error('No permission to update ' + targetPath)
+      }      
       const provider = await this.call('fileManager', 'getProviderOf', url)
       if (provider) {
         if (provider.type === 'localhost' && !provider.isConnected()) {
@@ -177,7 +181,7 @@ export class CompilerImports extends Plugin {
         }
       }
     } catch (e) {
-      throw new Error(`not found ${url}`)
+      throw new Error(e)
     }
   }
 }

libs/remix-ui/permission-handler/src/interface/index.ts

@@ -5,7 +5,8 @@ export interface PermissionHandlerValue {
     to: Profile,
     remember: boolean,
     method: string,
-    message: string
+    message: string,
+    sensitiveCall: boolean
 }
 
 export interface PermissionHandlerProps {

libs/remix-ui/permission-handler/src/lib/permission-dialog.tsx

@@ -3,7 +3,7 @@ import { PermissionHandlerProps } from '../interface'
 import './permission-dialog.css'
 
 const PermissionHandlerDialog = (props: PermissionHandlerProps) => {
-  const { from, to, remember, method, message } = props.value
+  const { from, to, remember, method, message, sensitiveCall } = props.value
   const [feedback, setFeedback] = useState<string>('')
   const theme = props.theme
 
@@ -52,12 +52,14 @@ const PermissionHandlerDialog = (props: PermissionHandlerProps) => {
       <h6>{to.displayName} :</h6>
       <p> {to.description || <i>No description Provided</i>}</p>
       {pluginMessage()}
+      { sensitiveCall ? <p className='text-warning'><i className="fas fa-exclamation-triangle mr-2" aria-hidden="true"></i>You are going to process a sensitive call. Please make sure you trust this plugin.</p> : '' }
     </article>
     <article className='remember'>
-      <div className='form-check'>
+      { !sensitiveCall && <div className='form-check'>
         {rememberSwitch()}
         <label htmlFor='remember' className="form-check-label" data-id="permissionHandlerRememberChoice">Remember this choice</label>
         </div>
+      }
       <button className="btn btn-sm" onClick={reset}>Reset all Permissions</button>
     </article>
     <div>{feedback}</div>