Skip to content

Account for Windows and MSFT Edge limitations when loading WebViews - #42

Merged
breedloj merged 9 commits into
mainfrom
shruti0085/setupQ
Sep 27, 2024
Merged

Account for Windows and MSFT Edge limitations when loading WebViews#42
breedloj merged 9 commits into
mainfrom
shruti0085/setupQ

Conversation

@shruti0085

@shruti0085 shruti0085 commented Sep 26, 2024

Copy link
Copy Markdown
Contributor

Description of changes:
This change introduces some updates to account for development limitations when using WebViews in Eclipse with Windows and MSFT Edge as the backing browser technology.
Changes:

  • File permissions added to the node executable have been removed for Windows because it otherwise fails to load
  • SWT.Edge browser style is used for Windows (More details here)
  • Edge/Chrome does not support directly loading a local resource on disk into html. This is security limitation present in most modern browsers. To address this issue and for better future scalability and maintenance,
    • a virtual host mapping has been created for the locations on disk where js files are referenced in the html loaded with WebViews.
    • Jetty server is used to create virtual servers for toolkit login view and amazon q chat view respectively. Ports for each are auto-assigned by Jetty based on system availability.
    • Localhost is the only virtual host that works rn, will investigate with a customized name as a follow-up

In addition some security recommendations has been added for chat webview CSP meta

  • default-src 'none'; - By default, do not allow any resources to load
  • script-src {javascriptFilePath} 'unsafe-inline' - Allow inline JS to load in http://{virtualhost}/amazonq-ui.js, which is required for mynah-ui to run
  • style-src {javascriptFilePath} 'unsafe-inline'; - Allow inline css to load in http://{virtualhost}/amazonq-ui.js, which is required for mynah-ui to run
  • img-src 'self' data: - Allow inline images (svg in particular) that start with data: to load in the same origin
  • object-src 'none'; base-uri 'none'; recommended by security internally
  • Added upgrade-insecure-requests to CSP

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Comment on lines +66 to +68
if (platform == PluginPlatform.WINDOWS) {
return;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's try checking for a POSIX compatible file system for a more robust check:

    public static boolean hasPosixFilePermissions(Path path) {
        return path.getFileSystem().supportedFileAttributeViews().contains("posix");
    }

if (platform == PluginPlatform.WINDOWS) {
return SWT.EDGE;
}
return SWT.NATIVE;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's make the default explicitly SWT.WEBKIT. The other types seem to be deprecated but we don't want to leave the mapping up to chance.

* @param jsPath
* @return server launched
*/
protected Server setupVirtualServer(final String jsPath) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My preference would be for this piece to live in a separate class that abstracts away the internal Server - could be WebviewAssetServer with a resolve(path) method (and stop).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the idea. Updated

protected Server setupVirtualServer(final String jsPath) {
Server server = null;
try {
server = new Server(0);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! Glad this worked.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants