1722966 - Cannot install manifest v2 extension with host_permissions or action key in manifest file

Reporter

Description

•

4 years ago

We've got a user report that this extension cannot be loaded on Nightly: https://addons.mozilla.org/en-US/firefox/addon/omnivore/

This extension has "manifest_version": 2, in manifest.json and the following keys:

  "permissions": [
    "activeTab",
    "storage",
    "scripting",
    "https://*/**",
    "http://*/**"
  ],
  "host_permissions": [
    "https://*/**",
    "http://*/**"
  ],

host_permissions is unsupported in Manifest v2. The desired behavior is to not reject installation of extensions that use manifest v3 properties, but to just ignore the key and print a warning in the console.

Console output upon trying to install the extension:

 1627577008271	addons.webextension.<unknown>	ERROR	Loading extension 'null': Reading manifest: Property "host_permissions" is unsupported in Manifest Version 2
    append resource://gre/modules/Log.jsm:723
    log resource://gre/modules/Log.jsm:379
    error resource://gre/modules/Log.jsm:387
    _logMessage resource://gre/modules/Extension.jsm:560
    logError resource://gre/modules/Extension.jsm:556
    packagingError resource://gre/modules/Extension.jsm:543
    manifestError resource://gre/modules/Extension.jsm:529
    parseManifest resource://gre/modules/Extension.jsm:1001
    InterpretGeneratorResume self-hosted:1482
    AsyncFunctionNext self-hosted:692
    (Async: async)
    loadManifest resource://gre/modules/Extension.jsm:1294
    loadManifestFromWebManifest resource://gre/modules/addons/XPIInstall.jsm:454
    loadManifest resource://gre/modules/addons/XPIInstall.jsm:657
    InterpretGeneratorResume self-hosted:1482
    AsyncFunctionNext self-hosted:692
    (Async: async)
    loadManifest resource://gre/modules/addons/XPIInstall.jsm:1542
    onStopRequest resource://gre/modules/addons/XPIInstall.jsm:2460

Shane Caraveo (:mixedpuppy)

Comment 1

•

4 years ago

This also goes for "action" and maybe other v3 keys in v2 manifests.

Rob Wu [:robwu]

Reporter

Comment 2

•

4 years ago

On Beta, the warning is different, because host_permissions is not recognized on Beta. The extension can still not load because the extension (version 0.1.14) has the following manifest:

  "action": {
    "default_icon": {
      "16": "images/toolbar/icon-16.png",
      "19": "images/toolbar/icon-19.png",
      "24": "images/toolbar/icon-24.png",
      "32": "images/toolbar/icon-32.png",
      "38": "images/toolbar/icon-38.png"
    },
    "default_title": "Omnivore Save Article"
  },

addons.webextension.<unknown>	ERROR	Loading extension 'null': Reading manifest: Property "action" is unsupported in Manifest Version 2
    append resource://gre/modules/Log.jsm:723
    log resource://gre/modules/Log.jsm:379
    error resource://gre/modules/Log.jsm:387
    _logMessage resource://gre/modules/Extension.jsm:558
    logError resource://gre/modules/Extension.jsm:554
    packagingError resource://gre/modules/Extension.jsm:541
    manifestError resource://gre/modules/Extension.jsm:527
    parseManifest resource://gre/modules/Extension.jsm:995
    InterpretGeneratorResume self-hosted:1482
    AsyncFunctionNext self-hosted:692
    (Async: async)
    loadManifest resource://gre/modules/Extension.jsm:1286
    loadManifestFromWebManifest resource://gre/modules/addons/XPIInstall.jsm:454
    loadManifest resource://gre/modules/addons/XPIInstall.jsm:657
    InterpretGeneratorResume self-hosted:1482
    AsyncFunctionNext self-hosted:692
    (Async: async)
    loadManifest resource://gre/modules/addons/XPIInstall.jsm:1542
    onStopRequest resource://gre/modules/addons/XPIInstall.jsm:2460

status-firefox91: unaffected → affected

status-firefox-esr78: --- → unaffected

status-firefox-esr91: --- → affected

Regressed by: 1706398

Summary: Cannot install manifest v2 extension with host_permissions key in manifest file → Cannot install manifest v2 extension with host_permissions or action key in manifest file

BMO Automation

Updated

•

4 years ago

Has Regression Range: --- → yes

Tomislav Jovanovic :zombie

Assignee

Comment 3

•

4 years ago

We'll want to upload the "action" one to beta, so I'll it as two patches.

Also, without major jumping through hoops, we'll not be able to have a test for "host_permissions" because of the test helper that just merges that key into "permissions" for manifest version 2.

Assignee: nobody → tomica

Severity: -- → S2

Priority: -- → P1

Shane Caraveo (:mixedpuppy)

Comment 4

•

4 years ago

action is in beta, so this needs a fix and uplift. at least one addon on AMO has an action key.

See Also: → https://mozilla-hub.atlassian.net/browse/WEBEXT-119

Shane Caraveo (:mixedpuppy)

Comment 5

•

4 years ago

Sorry for the repeat comment.

Tomislav Jovanovic :zombie

Assignee

Comment 6

•

4 years ago

[Tracking Requested - why for this release]: Some existing extensions uploaded to/installed from AMO will stop working once users update to 91.

tracking-firefox-esr91: --- → ?

Luca Greco [:rpl] [:luca] [:lgreco]

Updated

•

4 years ago

Comment 7

•

4 years ago

Attached file Bug 1722966 - Warn and ignore new MV3 keys in existing MV2 extensions — Details

Tomislav Jovanovic :zombie

Assignee

Comment 8

•

4 years ago

https://treeherder.mozilla.org/jobs?repo=try&revision=4ed0e3fbe0956320f47aac205c7a2ad120b1b963

Shane Caraveo (:mixedpuppy)

Comment 9

•

4 years ago

Comment on attachment 9233783 [details]
Bug 1722966 - Warn and ignore new MV3 keys in existing MV2 extensions

Beta/Release Uplift Approval Request

User impact if declined: some addons (70+) released on AMO fail to install
Is this code covered by automated tests?: Yes
Has the fix been verified in Nightly?: No
Needs manual test from QE?: Yes
If yes, steps to reproduce: Install https://addons.mozilla.org/en-US/firefox/addon/omnivore/

It currently fails to install on beta. With this patch landed, it will install.

List of other uplifts needed: None
Risk to taking this patch: Low
Why is the change risky/not risky? (and alternatives if risky): Some addons have taken advantage of lax restrictions in our manifest handling and have included both manifest v2 and v3 keys. Till now we only warned on them. With some mv3 changes, we now throw an error, resulting in a failed install.

This change issues a console warning (rather than throwing an error) if an addon includes MV3 keys in its manifest, and strips those keys from the normalized manifest.

The change is minimal and has a test.

String changes made/needed: none

Attachment #9233783 - Flags: approval-mozilla-beta?

Shane Caraveo (:mixedpuppy)

Updated

•

4 years ago

Flags: qe-verify+

Pulsebot

Comment 10

•

4 years ago

Pushed by tjovanovic@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/cbf0b6867654 Warn and ignore new MV3 keys in existing MV2 extensions r=mixedpuppy,robwu

Ryan VanderMeulen [:RyanVM]

Updated

•

4 years ago

tracking-firefox91: --- → +

tracking-firefox92: --- → +

Noemi Erli[:noemi_erli]

Comment 11

•

4 years ago

Backed out changeset cbf0b6867654 (Bug 1722966) for causing failures in test_ext_schemas_versioned.js CLOSED TREE

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&collapsedPushes=842382&selectedTaskRun=YiClmi1rRz2lMguXgUF2Tg.0&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&revision=cbf0b6867654070dfaf8d72ca44aa26a7261517e

Log: https://treeherder.mozilla.org/logviewer?job_id=346824698&repo=autoland&lineNumber=5242

Backout: https://hg.mozilla.org/integration/autoland/rev/6c32884a3f9edb7bbc1325be5286f214de05111c

Flags: needinfo?(tomica)

Luca Greco [:rpl] [:luca] [:lgreco]

Comment 12

•

4 years ago

Tomislav and I took a look into why the xpcshell test is failing on Android but passes successfully on Android, and we identified that the different behavior is due to the different value set for the extensions.webextensions.warnings-as-errors pref on Desktop.

On Desktop, extensions.webextensions.warnings-as-errors is set to true (I think that the value set to true is coming from the user pref file here https://searchfox.org/mozilla-central/source/testing/profiles/xpcshell/user.js)
On Android (where it is set to false, which it may be due to not picking up that testing/profiles/xpcshell/user.js user prefs file, but I have to double-check if that is actually the case)

In my opinion, technically the android run is the one right, the test case is currently testing a test-only behavior instead of the behavior without that pref set as it was meant to do.

We will agree on how to update the patch to make the test to pass (and possibly test the behaviors that it is supposed to cover) and re-land the fix as soon as possible.

Flags: needinfo?(tomica)

Pulsebot

Comment 14

•

4 years ago

Pushed by tjovanovic@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/e2cba77bdbbc Warn and ignore new MV3 keys in existing MV2 extensions r=mixedpuppy,robwu,rpl

Shane Caraveo (:mixedpuppy)

Updated

•

4 years ago

Blocks: 1723156

Tomislav Jovanovic :zombie

Assignee

Updated

•

4 years ago

Blocks: 1723189

Rob Wu [:robwu]

Reporter

Updated

•

4 years ago

Comment 15

•

4 years ago

Added as a see also a link to the new addons-linter pull request that is meant to align the addons-linter behavior (and the validation on submissions to AMO):

https://github.com/mozilla/addons-linter/pull/3838

See Also: → https://github.com/mozilla/addons-linter/pull/3838

Sandor Molnar[:smolnar]

Comment 16

•

4 years ago

bugherder

https://hg.mozilla.org/mozilla-central/rev/e2cba77bdbbc

Status: NEW → RESOLVED

Closed: 4 years ago

status-firefox92: affected → fixed

Resolution: --- → FIXED

Target Milestone: --- → 92 Branch

Alex Cornestean

Comment 17

•

4 years ago

•

Edited

Hello,

Verified the fix on the latest Nightly (92.0a1/20210801214108) under Windows 10 x64 and Ubuntu 16.04 LTS.

The add-on can be properly installed from AMO (https://addons.mozilla.org/en-US/firefox/addon/omnivore/) and a warning is printed in the browser console, as per the desired behavior mentioned in the original description of the issue – see screenshot. No related errors are shown in the console either.

Status: RESOLVED → VERIFIED

status-firefox92: fixed → verified

Alex Cornestean

Comment 18

•

4 years ago

Attached image 2021-08-02_09h39_54.png — Details

Julien Cristau [:jcristau]

Comment 19

•

4 years ago

Comment on attachment 9233783 [details]
Bug 1722966 - Warn and ignore new MV3 keys in existing MV2 extensions

approved for 91 rc1

Attachment #9233783 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

Julien Cristau [:jcristau]

Comment 20

•

4 years ago

bugherder uplift

https://hg.mozilla.org/releases/mozilla-beta/rev/4f6659c93a9d

status-firefox91: affected → fixed

Flags: in-testsuite+

Ryan VanderMeulen [:RyanVM]

Updated

•

4 years ago

status-firefox-esr91: affected → ---

tracking-firefox-esr91: ? → ---

Alex Cornestean

Comment 21

•

4 years ago

Hello,

Verified the fix on the latest Beta (91.0/20210802144751) under Windows 10 x64 and Ubuntu 16.04 LTS.

The add-on can be properly installed from AMO (https://addons.mozilla.org/en-US/firefox/addon/omnivore/) and a warning is printed in the browser console, as per the desired behavior mentioned in the original description of the issue – see screenshot. No related errors are shown in the console either.

status-firefox91: fixed → verified

Flags: qe-verify+

Alex Cornestean

Comment 22

•

4 years ago

Attached image 2021-08-03_09h11_22.png — Details

Bug 1722966 - Warn and ignore new MV3 keys in existing MV2 extensions 4 years ago Tomislav Jovanovic :zombie 48 bytes, text/x-phabricator-request	jcristau : approval-mozilla-beta+	Details \| Review
2021-08-02_09h39_54.png 4 years ago Alex Cornestean 48.19 KB, image/png		Details
2021-08-03_09h11_22.png 4 years ago Alex Cornestean 46.20 KB, image/png		Details