1298060 - Allow reviewers to install add-ons without an application id in the manifest

Status: RESOLVED DUPLICATE of bug 1284407

(Toolkit :: Add-ons Manager, defect)

Description

[Andy McKay]

This is to resolve the issue found in: https://github.com/mozilla/addons-server/issues/3183

Reviewers install add-ons during the review process *before* they are signed, so they can see how the add-on performs. However if the add-on has no application id in the manifest, it can't be installed even if signing is turned off.

To make life easier for reviewers it was suggested that:
* if signing is turned off
* if the add-on has no id
* and its loaded as an XPI
* create a temporary id
* load it temporarily

This mirrors what happens in about:debugging right now, but about:debugging only provides that temporary id if its loaded from a directory. We felt that we should only load from an XPI when signing is turned off to try and not encourage people to use this as a signing workaround.

Comment 1

[Andrew Swan [:aswan]]

Two clarifications:
1. This is just for webextensions yes?  (I don't think "if the add-on has no id" applies to other types but just making sure)
2. Are you asking for all installation paths to (silently?) switch to temporary install under these conditions or just asking to get the existing bugs with temporary installation from xpis fixed?

Comment 2

[Dave Townsend [:mossop]]

I think we should only do this for temporary add-ons by fixing bug 1284407 unless there is a compelling reason otherwise.

Comment 3

[Andy McKay]

(In reply to Andrew Swan [:aswan] from comment #1)
> Two clarifications:
> 1. This is just for webextensions yes?  (I don't think "if the add-on has no
> id" applies to other types but just making sure)

Yes.

> 2. Are you asking for all installation paths to (silently?) switch to
> temporary install under these conditions or just asking to get the existing
> bugs with temporary installation from xpis fixed?

No. Just, temporary installation.

Comment 4

[Andy McKay]

(In reply to Dave Townsend [:mossop] from comment #2)
> I think we should only do this for temporary add-ons by fixing bug 1284407
> unless there is a compelling reason otherwise.

It does sound like we could dupe this to bug 1284407. The only difference in this bug is the suggestion to do it when signing is disabled. We were concerned that allowing XPIs to be loaded temporarily with signing enabled would be a concern, all it takes is one malicious temporary load of an add-on...

Comment 5

[Andrew Swan [:aswan]]

(In reply to Andy McKay [:andym] from comment #4)
> We were
> concerned that allowing XPIs to be loaded temporarily with signing enabled
> would be a concern, all it takes is one malicious temporary load of an
> add-on...

I don't understand the concern, its about a scenario where an attacker tricks a user into downloading an unsigned malicious xpi, then navigating to about:debugging, choosing "install temporary add-on", navigating to the download directory in the ensuing file picker and selecting that malicious xpi?

If we want to guard against that with a pref, I don't object, but I would favor creating a new pref rather than overloading the existing one.

Incidentally, why don't reviewers use unbranded builds?

Comment 6

[Andy McKay]

Andrew and I chatted about this and figured that fixing bug 1284407 is the best way to go. Assuming that's a bug that's causing it to fail and isn't something on purpose.