Closed Bug 1172028 Opened 5 years ago Closed 5 years ago

Only fully-signed add-ons should be offered as sideloaded.


(Toolkit :: Add-ons Manager, defect, P1)




Tracking Status
firefox40 + fixed
firefox41 + fixed


(Reporter: mossop, Assigned: mossop)



(Whiteboard: [hijacking][fxsearch])


(2 files)

No description provided.
Assignee: nobody → dtownsend
[Tracking Requested - why for this release]:

This was an error when we implemented add-on signing, sideloaded add-ons should only be usable if fully signed.
Flags: qe-verify-
Flags: firefox-backlog+
Whiteboard: [hijacking][fxsearch]
Rank: 9
Priority: -- → P1
Attached patch patchSplinter Review
This makes add-ons marked with foreignInstall unusable unless fully signed. It also increases the cases where we set that flag by including the staging directory unless there is a cached json manifest there, which broke a few of our tests suites!
Attachment #8616728 - Flags: review?(dveditz)
Comment on attachment 8616728 [details] [diff] [review]

Review of attachment 8616728 [details] [diff] [review]:

Attachment #8616728 - Flags: review?(dveditz) → review+
Tracking for 40 and 41 because it is affected.
sorry had to back this out in since this or the other cset caused a perma failure in xperf like
Flags: needinfo?(dtownsend)
Attached patch talos patchSplinter Review
The tp5n test overrides the default talos setting for extensions.autoDisableScopes causing Firefox to refuse to enable the talos extensions on startup with the changes in this patch. Clearing this solves it and I don't think there would be any ill-effects from this.

Interestingly it also clears extensions.enabledScopes which means the default theme is disabled so we're actually doing slightly different file I/O during startup in the xperf run than a real build does, might even be displaying different icons depending on the Windows version. Removing that causes the test to fail with a new file being accessed so I left it for now.
Flags: needinfo?(dtownsend)
Attachment #8621168 - Flags: review?(jmaher)
Comment on attachment 8621168 [details] [diff] [review]
talos patch

Review of attachment 8621168 [details] [diff] [review]:

thanks, I will land this on talos and we can update talos.json and reland.
Attachment #8621168 - Flags: review?(jmaher) → review+
Relanded with the updated talos revision:
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
Comment on attachment 8616728 [details] [diff] [review]

Approval Request Comment
[Feature/regressing bug #]: Add-on signing
[User impact if declined]: Preliminarily signed add-ons will be able to be sideloaded which wasn't the intention.
[Describe test coverage new/current, TreeHerder]: On m-c since last week with automated tests
[Risks and why]: Low risk, this is a straightforward change
[String/UUID change made/needed]: None
Attachment #8616728 - Flags: approval-mozilla-aurora?
Comment on attachment 8616728 [details] [diff] [review]

Potential source of trouble, taking the fix.
Attachment #8616728 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
I just uplifted m-b talos.json desktop pin to point to the same as m-c, and m-a in order to pick up a fix from:

sanity check, does m-b require too if I did:
Flags: needinfo?(dtownsend)
(In reply to Jordan Lund (:jlund) from comment #15)
> I just uplifted m-b talos.json desktop pin to point to the same as m-c, and
> m-a in order to pick up a fix from:
> sanity check, does m-b require
> too if I did:
> talos.json

We're not intending on uplifting this to beta so it shouldn't need it.
Flags: needinfo?(dtownsend)
You need to log in before you can comment on or make changes to this bug.