1778461 - Normalize and grant apropriate "Access to all your data" origin permissions

Status: VERIFIED FIXED

(WebExtensions :: Untriaged, defect, P2)

Description

[ileturia]

Attachment: Example of a very simple manifest.json and background.js to reproduce the error

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:104.0) Gecko/20100101 Firefox/104.0

Steps to reproduce:

In an extension, when clicked I need to open a new tab with a URL and inject some CSS and JS into it.

Actual results:

It gives a "Missing host permission for the tab" error. This is becaude the URL of the new tab is "about:blank" (even given it plenty of time to load, or even running it after "completed" status change).

Expected results:

It should inject the CSS and JS without problems, because there are appropriate host_permissions for the URL, but not for "about:blank" because it is not possible.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

The Bugbug bot thinks this bug should belong to the 'WebExtensions::Untriaged' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Comment 2

[ileturia]

It's the same with browser.tabs.update.

Comment 3

[Tomislav Jovanovic :zombie]

Check if this is related to ungranted host permission or activeTab.

Comment 4

[Tomislav Jovanovic :zombie]

Can you please provide the extension and STR to reproduce this issue?

This is might be an issue with host permissions, in MV3 they are not granted by default on install, user needs to grant them at some point, though it looks like this should work with activeTab.

Comment 5

[ileturia]

(In reply to Tomislav Jovanovic :zombie from comment #4)

Can you please provide the extension and STR to reproduce this issue?

It is not in AMO, it is a self-hosted extension.

This is might be an issue with host permissions, in MV3 they are not granted by default on install, user needs to grant them at some point, though it looks like this should work with activeTab.

The problem is not when a user installs it, it happens also when developing installed with "web-ext run". And activeTab is in the manifest.

Comment 6

[Rob Wu [:robwu]]

The STR from the report is invalid. It's missing the version, name and action manifest.json keys, and there is a trailing comma in the host_permissions array. After fixing up in manifest.json (i.e. starting manifest.json with the content below), I can reproduce the problem.

{
    "version": "1",
    "name": "str here",
    "action": {},

Note that activeTab is not expected to work, because a new tab is opened, and action.onClicked only applies to the current tab at the time of clicking the action button.

I visited about:addons and granted host permissions, which did still not work. I ran the following snippet, and found that http://*/* was granted but https://*/* was not.

WebExtensionPolicy.getActiveExtensions().find(p=>p.debugName.includes("str")).allowedOrigins.patterns.map(p=>p.pattern)
result: Array [ "moz-extension://afe375e2-04c3-43ef-ba17-1931363da89f/*", "http://*/*" ]

Manually editing the tab URL to http://example.org instead of https caused the test to pass.

TL;DR: Allowing access to all URLs in about:addons does not grant https and http, even though it should. Possibly related to bug 1745820.

Comment 7

[Rob Wu [:robwu]]

Please provide the STR to exactly reproduce your problem. The title of the bug, the test case and the description of the bug are not matching up.

It could be an incorrect expectation, or an actual bug, but it is not possible to tell which one is applicable. In comment 6, I mentioned some of the possible causes of the observed result, but none of that is related to about:blank (your test case does not have anything that related to about:blank because you're running executeScript at a 5-second timer, at which point example.org has loaded already).

Comment 8

[ileturia]

I am sorry, I am not familiar with Mozilla development procedures, and I do not know what an STR is...

The manifest.json provided was incomplete, I just wanted to show the fields I thought were relevant (host_permissions, permissions, etc.).

You say that "http:/// was granted but https:/// was not", and maybe that is the problem, because the manifest.json asks for the HTTPS permission, but if it is not granted...

The reason for running executeScript at a 5-second timer is for giving the tab enough time to load everything before injecting the script. But it still gives the error. And I thought it might be because the URL property of the tab returns about:blank (both at the moment of creation and even 5 seconds later when everything is loaded) and the plugin has not permissions for accessing that URL.

Comment 9

[Tomislav Jovanovic :zombie]

(In reply to Rob Wu [:robwu] from comment #6)

TL;DR: Allowing access to all URLs in about:addons does not grant https and http, even though it should. Possibly related to bug 1745820.

This was the behavior from before bug 1745820, we only show one "Access your data for all sites" toggle, which most of the times doesn't matter much (because it's usually *:/*/* and <all_urls>, but in this case extension has http:/*/* and https:/*/*, and we only show/grant the first one.

We've already noticed something related (see bug 1766055), but this seems like a more sharp corner case.

Comment 10

[Tomislav Jovanovic :zombie]

The title is a bit generic, but what we probably want is:

• if there are multiple "all sites" permissions in the manifest, when the user (manually) grants it, if all of them are subsumed by one, grant that one (for example, <all_urls>, *://*/*, and https://*/*).
• otherwise, grant (and remove) all of them (this case).

Comment 11

[ileturia]

Attachment: Files of a simple extension to reproduce the error

Comment 12

[ileturia]

Attached new file, with a complete and working extension to reproduce the error.

Comment 13

[ileturia]

The error occurs even if there is only one "all sites" permission in the manifest, no matter if it is <all_urls>, :///* or https:///

Comment 14

[Rob Wu [:robwu]]

(In reply to ileturia from comment #12)

Attached new file, with a complete and working extension to reproduce the error.

Please provide the full reproduction steps, including all manual actions that you are taking. Make sure to explicitly include when you're waiting (e..g 5 seconds) and where you are looking, along with the expected behavior and the actual behavior.

I identified one bug in comment 6, which is why the bug was renamed in comment 10. It's however not clear what part of the extension is considered to be a bug by you.

Comment 15

[ileturia]

Steps to reproduce the error:

• Download attachment #9285911 and unzip it.
• Open Firefox, go to about:addons, go to "Debug Add-ons", click on "Load Temporary Add-on" and select manifest.json of unzipped folder.
• Click on "Inspect".
• Click on add-on button in address bar (the image is a puzzle piece). A tab will open pointing to https://example.org/, and the dev tools tab will show a message like "Created new tab: 4, about:blank" ("about:blank" is the URL reported by the tab, tab.url, in the onCreated event).
• Wait 5 seconds.
• A timeout event will fire, the tab should be comlpletely loaded. The dev tools tab will show a message like "Created new tab: 4, about:blank", meaning that the URL property of the tab is still "about:blank" instead of "https://example.org/".
• The timeout event will also try to run executeScript to inject a very simple JS code in the newly created tab, but the dev tools tab will show an error message: "Error: Missing host permission for the tab".

Comment 16

[Alex Cornestean]

Hello,

I reproduced the issue on the latest Nightly (104.0a1/20220721214008), Beta (103.0/20220718155818) and Release (102.0.1/20220705093820) under Windows 10 x64.

Proceeding with the STR from Comment 15, clicking on the browser action will display one “Created new tab: 4, about:blank” message in the dev tools console and after a few seconds, another “Created new tab: 4, about:blank” message followed by “Error: Error: Missing host permission for the tab”.

For further details, see the attached screenshot.

Comment 17

[Alex Cornestean]

Attachment: 2022-07-22_12h21_31.png

Comment 18

[Rob Wu [:robwu]]

(In reply to ileturia from comment #15)

Steps to reproduce the error:

• Download attachment #9285911 and unzip it.
• Open Firefox, go to about:addons, go to "Debug Add-ons", click on "Load Temporary Add-on" and select manifest.json of unzipped folder.
• Click on "Inspect".
• Click on add-on button in address bar (the image is a puzzle piece). A tab will open pointing to https://example.org/, and the dev tools tab will show a message like "Created new tab: 4, about:blank" ("about:blank" is the URL reported by the tab, tab.url, in the onCreated event).
• Wait 5 seconds.
• A timeout event will fire, the tab should be comlpletely loaded. The dev tools tab will show a message like "Created new tab: 4, about:blank", meaning that the URL property of the tab is still "about:blank" instead of "https://example.org/".

This expectation is incorrect. The tab that you're logging is the state of the tab from 5 seconds before, i.e. when the tab was just created (without having finished loading yet). The tab object is not automatically updated; you'd have to use browser.tabs.get to get the latest known state of the tab.

• The timeout event will also try to run executeScript to inject a very simple JS code in the newly created tab, but the dev tools tab will show an error message: "Error: Missing host permission for the tab".

This is also expected - your extension does not have access to the tab. The host permissions have not been granted (because in MV3 host permissions are not granted by default, and you did not explicitly include a manual action to enable host permissions in your reproduction steps). Moreover, the activeTab permission does not apply because it would have granted access to the current tab, and not the new tab that you've opened. (as mentioned in comment 6)

Comment 19

[ileturia]

Attachment: executeScript-on-new-tab-error.zip

Comment 20

[ileturia]

(In reply to Rob Wu [:robwu] from comment #18)

(In reply to ileturia from comment #15)

Steps to reproduce the error:

• Download attachment #9285911 and unzip it.
• Open Firefox, go to about:addons, go to "Debug Add-ons", click on "Load Temporary Add-on" and select manifest.json of unzipped folder.
• Click on "Inspect".
• Click on add-on button in address bar (the image is a puzzle piece). A tab will open pointing to https://example.org/, and the dev tools tab will show a message like "Created new tab: 4, about:blank" ("about:blank" is the URL reported by the tab, tab.url, in the onCreated event).
• Wait 5 seconds.
• A timeout event will fire, the tab should be comlpletely loaded. The dev tools tab will show a message like "Created new tab: 4, about:blank", meaning that the URL property of the tab is still "about:blank" instead of "https://example.org/".
  This expectation is incorrect. The tab that you're logging is the state of the tab from 5 seconds before, i.e. when the tab was just created (without having finished loading yet). The tab object is not automatically updated; you'd have to use browser.tabs.get to get the latest known state of the tab.

OK, I have changed the code accordingly and now this does not happen, the URL it is showing now is the correct one.

• The timeout event will also try to run executeScript to inject a very simple JS code in the newly created tab, but the dev tools tab will show an error message: "Error: Missing host permission for the tab".
  This is also expected - your extension does not have access to the tab. The host permissions have not been granted (because in MV3 host permissions are not granted by default, and you did not explicitly include a manual action to enable host permissions in your reproduction steps). Moreover, the activeTab permission does not apply because it would have granted access to the current tab, and not the new tab that you've opened. (as mentioned in comment 6)

I am sorry, I do not understand this. I do have included host_permissions in manifest.json for https:///, is not that enough? When you say "you did not explicitly include a manual action to enable host permissions in your reproduction steps", you mean there is something else to do? How should this be done, or what would be the changes to do in the code? If there is something I am doing wrong in the procedure of creating a new tab and injecting a script into it in MV3, I would appreciate an example of the correct code to do so in it, since I have not found any...

Comment 22

[Rob Wu [:robwu]]

There is an obvious STR in bug 1797943.

This issue happens because:

1. The first "allUrls" permission is selected at https://searchfox.org/mozilla-central/rev/968bd894205cf4f579d94ac4e175cc3187458605/toolkit/components/extensions/Extension.jsm#1887-1888
2. and then associated as the only origin permission with https://searchfox.org/mozilla-central/rev/968bd894205cf4f579d94ac4e175cc3187458605/toolkit/components/extensions/Extension.jsm#2140
3. and finally used to generate the UI (list) at https://searchfox.org/mozilla-central/rev/968bd894205cf4f579d94ac4e175cc3187458605/toolkit/mozapps/extensions/content/aboutaddons.js#2581-2584

This implementation is inherently flawed. If an extension requests http://*/ and https://*/*, then the origin should be upgraded to *://*/* at the least.

And for cases where there is no easy consolidation, step 2 should be able to somehow match multiple permissions at once (e.g. http://*/* + wss://*/* should not grant access to https://*/*).

Note that this is about the allUrls case. We should also verify the behavior with non-wildcard permissions (e.g. https://example.com/* + http://example.com/*).

This looks like a serious bug that prevents effective adoption of MV3. Tom, could you take a look at this please?

Comment 23

[Rob Wu [:robwu]]

(In reply to Rob Wu [:robwu] from comment #22)

Note that this is about the allUrls case. We should also verify the behavior with non-wildcard permissions (e.g. https://example.com/* + http://example.com/*).

This is a known issue and explicitly referred to in the code as a TODO + bug 1765828, at https://searchfox.org/mozilla-central/rev/968bd894205cf4f579d94ac4e175cc3187458605/toolkit/components/extensions/Extension.jsm#1858-1859

Comment 24

[Rob Wu [:robwu]]

For comparison, Safari's behavior is mentioned at https://github.com/w3c/webextensions/issues/304#issuecomment-1293986379

Granted permissions from the UI are always *://*.domain/ patterns that are formed by what the extension asks for in the manifest. I think the only exception is origins directly requested from the extension via the permissions.request() API, those are as-is currently.

Comment 25

[Tomislav Jovanovic :zombie]

Attachment: Bug 1778461 - Fix handling of multiple "all sites" permissions in about:addons, r=robwu,rpl

Comment 26

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/699c070d4fba
Fix handling of multiple "all sites" permissions in about:addons, r=robwu

Comment 27

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/699c070d4fba

Comment 28

[Alex Cornestean]

Verified as Fixed on the latest Nightly (109.0a1/20221209044848). Tested on Windows 10 x64 and macOS 11.3.1.

Using the extension and STR provided in Bug 1797943, loading https://mozilla.org will immediately show the border i.e. content script is injected, as per the expected results mentioned in the bug report, confirming the fix.