[bug] The mini menu of windows webview still exists in production mode

[cangSDARM]

Describe the bug

If some text selected, the windows webview (edge) will show a mini menu like the image shows. This is not too important in development mode, but in production mode this could lead to some potential bugs that could compromise the app. Webview2 seems to have related commands that can be prohibited, but no configuration is displayed in tauri.

Relative issue:
MicrosoftEdge/WebView2Feedback#2140
wailsapp/wails#1188

Reproduction

build a windows app

Expected behavior

No response

Platform and versions

Operating System - Windows, version 10.0.19044 X64
Webview2 - 99.0.1150.55
Visual Studio Build Tools:
   - Visual Studio Community 2019

Node.js environment
  Node.js - 14.17.6
  @tauri-apps/cli - 1.0.0-rc.5 (outdated, latest: 1.0.0-rc.8)
  @tauri-apps/api - Not installed

Global packages
  npm - 6.14.11
  pnpm - 6.23.6
  yarn - 1.22.17

Rust environment
  rustup - 1.24.3
  rustc - 1.59.0
  cargo - 1.59.0
  toolchain - stable-x86_64-pc-windows-msvc 

App directory structure
/.git
/.parcel-cache
/client
/dist
/node_modules
/server

App
  tauri - 1.0.0-rc.6
  tauri-build - 1.0.0-rc.5
  tao - 0.7.0
  wry - 0.14.0
  build-type - bundle
  CSP - default-src 'self'
  distDir - ../dist
  devPath - http://localhost:3000/
  framework - React

Stack trace

No response

Additional context

No response

[amrbashir]

The proposed workaround in the issue you linked, doesn't work so we need to wait for webview2 team to add a proper api.

[JensMertelmeyer]

The proposed workaround in the issue you linked, doesn't work

It does work. You can just add an environment variable to your process and the WebView2 runtime on Windows will pick it up.
Like this:

fn disable_webview2_mini_menu() {
    let key = "WEBVIEW2_ADDITIONAL_BROWSER_ARGUMENTS";
    let value = "--disable-features=msWebOOUI,msPdfOOUI";
    std::env::set_var(key, value);
}

Keep in mind that this must be done before the WebView is being constructed.

[nothingismagick]

Allowing environment variables to influence runtime behaviour is extraordinarily dangerous. There are a whole suite of attack vectors in electron that leverage this chromium "feature".

[nothingismagick]

I understand your solution is being done within the internal loop, but we generally prefer APIs. Maybe @wravery has an idea here.

[JensMertelmeyer]

Of course this is a workaround and not a solution to the problem.

I don't want to derail this topic, but can you give me a pointer how the three lines above present a security issue? I am aware that sensitive information should not be stored in an environment variable. Here were are just adding a flag in the scope of our own process. I fail to see how this is extraordinarily dangerous. Anybody can even launch an already existing binary with those environment variables already in place.

[amrbashir]

It does work. You can just add an environment variable to your process 

I deliberately ignored this solution because anyone can do it in their app and because the security risks that might be involved.

There was another solution in the linked issue by using AdditionalBrowserArguments when initializing Webview2 which is more secure than env var, but it didn't work the last time I tried it. There is a chance I might've messed up so I will give it another try some time next week.

[nothingismagick]

WRT environment variables in your snippet (its a trigger for me), but I just think that it's a general control structure that should be avoided. We are trying to build hardened apps. When envvars impact a binary's runtime behavior, you are entering the hellscape of unknowable behaviour. Sure, this might be a risk you are comfortable with because they have been assessed properly and mitigated with policy etc. And don't get me wrong, having configurable runtimes can be utilitarian, in very certain circumstances. But generally its a terrible idea for apps you ship to normal consumers, because its a simple attack vector. Similarly, holding your ssh private key in /home/username/.ssh means that anyone who can break into your filesystem basically owns you. The point is, that you SHOULD be a control freak when it comes to security and locking down the thing you ship to really only be able to do what you want it to do, in the way you want it.

My general rule of thumb is that convention should always supercede configuration at runtime.

Anyway, if you want a sobering watch about something tangential:

https://youtu.be/eTcVLqKpZJc

And this is a good summary about why NOT trusting even benign ENVVARS is smart:

https://security.stackexchange.com/questions/119962/what-are-some-vulnerabilities-of-environment-variables-on-any-platform

[wravery]

There was another solution in the linked issue by using AdditionalBrowserArguments when initializing Webview2 which is more secure than env var, but it didn't work the last time I tried it. There a chance I might've messed up so I will give it another try some time next week.

That's the API I would have suggested. If the environment variable is working in testing, then I expect this to work too, I think it is equivalent at runtime for the sub-processes.

[JensMertelmeyer]

Good news - It also works when using CreateCoreWebView2EnvironmentWithOptions(..) instead of using environment variables 😊

I am very new to Rust, maybe I forgot something obvious? Thanks for taking the time to check it out.