754915 - Web App runtime should auto-disable profile-loaded addons and disable add-on installation via the web

Status: VERIFIED FIXED

(Firefox Graveyard :: Webapp Runtime, defect)

Description

[Myk Melez [:myk] [@mykmelez]]

Webapps shouldn't load profile-installed addons, as support for addons is not yet in scope for the webapp runtime, and an addons feature has not been researched and designed.  The only exception is the addon installed by the unit test framework that adw is implementing in bug 733631, which should work so we can haz unit tests.

Bug 748967 disables globally-installed addons using the extensions.enabledScopes preference.  Based on my primitive understanding of extensions.enabledScopes and .autoDisableScopes, I don't think there's a combination of their values that would fix this bug, but cc:ing mossop for confirmation.

Comment 1

[Jason Smith [:jsmith]]

Flagging as a blocker for 1st release of desktop web runtime - we can't enable add-ons in web app profiles for the 1st release of the desktop web runtime.

Comment 2

[:Gavin Sharp [email: [e-mail]]]

This isn't a blocking issue, since there's no exposed way to install add-ons in the WebAppRT profile. I'm not even sure we need to bother fixing this.

Comment 3

[Dave Townsend [:mossop]]

autoDisableScopes can make it so any dropped in add-ons don't enable, but yeah there is no way to disable the profile location completely (especially not ignoring the test add-ons). It would be simpler to disable the add-ons manager entirely I suspect, but sounds like this should be a big deal anyway.

Comment 4

[Jason Smith [:jsmith]]

See https://wiki.mozilla.org/Kilimanjaro/ProductDraft#You_will_be_able_to_install_and_use_your_apps_across_phones_and_PCs_where_WebRT_is_available. The product draft indicates that "Firefox add-ons do not get executed when running an app" as a P1 requirement for k9o. Could you clarify why this does not fit that user story?

I'll move this bug back to untriaged until I get more clarification.

Comment 5

[Jason Smith [:jsmith]]

Extension to comment 4 - What about use cases involving a malicious user with profile-loaded addons? Are there any risks in this area?

Comment 6

[Dave Townsend [:mossop]]

(In reply to Jason Smith [:jsmith] from comment #4)
> See
> https://wiki.mozilla.org/Kilimanjaro/
> ProductDraft#You_will_be_able_to_install_and_use_your_apps_across_phones_and_
> PCs_where_WebRT_is_available. The product draft indicates that "Firefox
> add-ons do not get executed when running an app" as a P1 requirement for
> k9o. Could you clarify why this does not fit that user story?
> 
> I'll move this bug back to untriaged until I get more clarification.

The WebApp runtime runs with a different profile to Firefox so it doesn't have any of Firefox's add-ons in its profile. Bug 748967 will take care of stopping any other add-ons on the system from running.

Comment 7

[:Gavin Sharp [email: [e-mail]]]

I don't see why this needs any information from the product team. If we're going to fix this at all we'll probably just want to disable the add-ons manager entirely in the WebAppRT, but we only need to do that if the benefits (startup time?) outweigh the costs (need a more complicated testing solution?).

Comment 8

[Blair McBride [:Unfocused] (UNAVAILABLE)]

I looked into what would be needed to disable the Add-ons Manager - disabling it is easy enough (bug 671894 has a patch that helps), the main issue is with the numerous components that use it. Thankfully the blocklist service interacts directly with the plugin host, so the Add-ons Manager parts can be disabled without affecting blocklisting of plugins. LightweightThemeManager is more awkward since it also acts as an add-on type provider - its doable, but messy. Sync's add-on engine can presumably just be disabled entirely. I assume the update service won't be enabled, but if it is then that will be "interesting" figuring out the right behaviour (since it checks addon compatibility in the current profile, but it should do that for Firefox's profile). The plugin wizard relies on the Add-on Manager to install plugins, but it's not the most wonderful of experiences anyway.

Those components that normally use the Add-ons Manager would ideally have separate tests for when it's disabled. Which of course can't be done as part of the test suite for the main application.

As for the add-on for the test framework, the script that starts tests could just manually write the relevant entry into extensions.ini (loading that isn't handled by the Add-ons Manager).


If we *don't* disable the Add-ons Manager:
* Startup impact should be negligible on desktop, though it can make a noticeable difference on mobile, depending on the device (bug 746916 and followups will help with that). Changes to addons have a relatively large impact due to I/O, but disabling all but the profile scope and disabling installs should make that a non-problem.
* I don't have numbers for memory impact - but again mobile will be affected more. We do know SQLite takes up more memory that we'd like (see bug 726556), but the database is lazy-loaded only when needed (but the periodic blocklist check will do that once a day sometime after startup). Bug 746916 and followups will also reduce general memory consumption.
* xpinstall.enabled should be set to false, even if only to prevent addon installs from AMO
* Bug 748967 (setting extensions.enabledScopes) achieves the goal as stated in the k9o product draft. Setting xpinstall.enabled and extensions.autoDisableScopes achieves the extra goal of disabling any addon installs.

Comment 9

[:Gavin Sharp [email: [e-mail]]]

(In reply to Blair McBride (:Unfocused) from comment #8)

Thanks for looking into this, Blair!

It sounds like for now we should just set the xpinstall.enabled=false and extensions.autoDisableScopes=1 prefs as you suggest - easy to do and it will stop the "drop in an XPI in the webapp profile" as well as the "manage to get the webapprt to navigate to AMO" scenarios, as I understand it.

I want to investigate disabling more components in the WebAppRT separately, but given the complications you've identified with doing that in this case, we shouldn't block on it.

Comment 10

[:Gavin Sharp [email: [e-mail]]]

Attachment: patch

I've not tested this yet, but this should do the trick.

Comment 11

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Comment on attachment 625752 [details] [diff] [review]
patch

Yep.

Note: If the test framework addon is installed the normal way, extensions.autoDisableScopes will need to be flipped to allow addons in the profile by default. This is what the existing test frameworks do.

Comment 12

[:Gavin Sharp [email: [e-mail]]]

Confirmed with local testing that this fixes the "drop-in XPI" scenario.

Comment 13

[:Gavin Sharp [email: [e-mail]]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/023f75b6a2ec

Comment 14

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/023f75b6a2ec

Comment 15

[Jason Smith [:jsmith]]

Verified by doing the following:

1. Used Andy's hack for webapp.json for an installed app to point the origin to about:addons
2. Launch the application
3. Select install add-on from file
4. Install gavin's alert add-on

Result before patch (5/23 nightly build): Add-on successfully installs and fires alert on each start-up of the application.

Result after patch (5/24 nightly build): Add-on appears on the extensions list as downloaded, but it remains halted and unable to install the add-on.