Allow modifying privileged sites like AMO with an explicit host permission, separate from <all_urls> or activeTab

NEW
Unassigned

Status

P5
normal
2 years ago
3 months ago

People

(Reporter: kmag, Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: triaged)

(Reporter)

Description

2 years ago
This will require modifying the AMO validator to require admin review before signing such extensions.

Can someone please file an AMO bug to implement that behavior? The current list of sites protected by this policy are[1]:

  addons.mozilla.org
  discovery.addons.mozilla.org
  testpilot.firefox.com

[1]: http://searchfox.org/mozilla-central/rev/8fa84ca6444e2e01fb405e078f6d2c8da0e55723/toolkit/mozapps/extensions/AddonManagerWebAPI.cpp#23-25
So, whenever a webextension uses one or more of the mentioned urls in their host permissions, the review should automatically be flagged for admin review?
What do we do with unlisted add-ons?

Comment 2

2 years ago
I think this would be more maintainable if we made it a regular named permission instead of overloading host permissions.

Updated

2 years ago
Priority: -- → P2
Whiteboard: triaged
(Reporter)

Comment 3

2 years ago
(In reply to Andreas Wagner [:TheOne] from comment #1)
> So, whenever a webextension uses one or more of the mentioned urls in their
> host permissions, the review should automatically be flagged for admin
> review?

Correct.

> What do we do with unlisted add-ons?

Probably the same. Or just reject the add-on. I don't have a particularly strong opinion on which.

(In reply to Andrew Swan [:aswan] from comment #2)
> I think this would be more maintainable if we made it a regular named
> permission instead of overloading host permissions.

I don't think this would be tantamount to overloading host permissions. We'd essentially just be excluding those hosts from <all_urls> and activeTab, and require them to be explicitly specified.

Comment 4

2 years ago
(In reply to Kris Maglione [:kmag] from comment #3)
> I don't think this would be tantamount to overloading host permissions. We'd
> essentially just be excluding those hosts from <all_urls> and activeTab, and
> require them to be explicitly specified.

Well I have a few concerns:
- in permissions prompts we collapse redundant permissions so for instance if you request "<all_urls>" and "https://site.com", the prompt just says "all sites".  I could see somebody reasonably wanting to do similar collapsing elsewhere, but doing this with host permissions would prevent doing that.
- related to the above, when computing changes in permissions between updates, we don't currently do anything very clever with host permissions but i could see wanting to do so (e.g., going from <all_urls> in version 1 to "https://site.com" in version 2 is actually not requesting new permissions).  if some individual host permissions are treated specially, that would become more complicated
- now we have to keep the list of affected sites in sync between the browser and AMO, that's a drag
(Reporter)

Comment 5

2 years ago
(In reply to Andrew Swan [:aswan] from comment #4)
> Well I have a few concerns:
> - in permissions prompts we collapse redundant permissions so for instance
> if you request "<all_urls>" and "https://site.com", the prompt just says
> "all sites".  I could see somebody reasonably wanting to do similar
> collapsing elsewhere, but doing this with host permissions would prevent
> doing that.

Given that these extensions will require special review, I think that continuing to collapse them in the permission prompt is fine. Users shouldn't need to be aware that this is a special case, but reviewers and internal code will.

> - related to the above, when computing changes in permissions between
> updates, we don't currently do anything very clever with host permissions
> but i could see wanting to do so (e.g., going from <all_urls> in version 1
> to "https://site.com" in version 2 is actually not requesting new
> permissions).  if some individual host permissions are treated specially,
> that would become more complicated

Again, this shouldn't be an issue as far as updates or prompts are concerned.

> - now we have to keep the list of affected sites in sync between the browser
> and AMO, that's a drag

It is, but I can live with it.

Updated

2 years ago
Duplicate of this bug: 1342379

Comment 7

2 years ago
My bug has been m a duplicate of this one, but also concerns about:* pages.

Is the permission being discussed here intended to allow modification of about:* pages as well as AMO? Or is this marked as duplicate because there's not a "related" button?

Comment 8

2 years ago
Phone keyboard lost some characters:

> My bug has been marked as a duplicate of this one, but also concerns about:* pages...

Is what I meant to say.

Comment 9

2 years ago
about: pages were covered separately in bug 1270412
(Reporter)

Updated

2 years ago
Duplicate of this bug: 1344095
Duplicate of this bug: 1397332

Updated

10 months ago
Flags: needinfo?(amckay)

Comment 12

10 months ago
One use case was from reviewers, however the review tools on AMO have moved to a seperate domain, that's no longer relevant.

Most of the other requests seem to be around the ability to alter AMO (e.g. gestures) or the fact that people install an add-on from AMO and then test it on AMO because it was the place they were when they installed it.

It would be nice to fix, but I don't think it warrants a P2 and I'm dropping down the priority, although I'm tempted to close.

I'm also strongly against anything that requires "special review", any API needing that should provide a large amount of functionality to justify the work it entails.

Noting that Chrome can't run content scripts on the Chrome store either.
Flags: needinfo?(amckay)
Priority: P2 → P5
(In reply to Andy McKay [:andym] from comment #12)
> Most of the other requests seem to be around the ability to alter AMO (e.g.
> gestures) or the fact that people install an add-on from AMO and then test
> it on AMO because it was the place they were when they installed it.
Could this be solved by user messaging, e.g. a note somewhere that some add-ons will not work on AMO?

Updated

3 months ago
Product: Toolkit → WebExtensions
You need to log in before you can comment on or make changes to this bug.