741812 - [Security Review][Action Item]Marionette - AMO Review Information

Status: RESOLVED FIXED

(addons.mozilla.org Graveyard :: Add-on Validation, defect)

Description

[Curtis Koenig [:curtisk-use curtis.koenig+bzATgmail.com]]]

add verificaiton checking for AMO reivewers

Comment 1

[Malini Das [:mdas] - Away, not checking bugmail]

We are not building or packaging marionette in any of the firefox builds we release, so add-ons (and any other script) will not have access to any of the modules at this time. We are only building marionette for B2G builds by default, which is fine for now as they use it for testing.

Once we migrate more of our Firefox testing tools to Marionette, we may start packaging parts of Marionette in release builds. At that point, we should add verification checking.


How should we keep track of this bug for when it becomes relevant for Firefox?

Comment 2

[Curtis Koenig [:curtisk-use curtis.koenig+bzATgmail.com]]]

I am going to close this won'tfix for now, if/when we do put this stuff in firefox we can reopen this. Is there a bug I could put this as blocking so we don't lose all track of this?

Comment 3

[Daniel Veditz [:dveditz]]

We are now considering shipping this in optimized builds (see bug 870445) so this bug is valid again. AMO reviewers should watch for addons which
  * manipulate these prefs
  * attempt to use the marionette APIs

There may well be legitimate testing add-ons that use these so they should not be prohibited, we just need to make sure that they aren't used secretly from an add-on for which it's irrelevant and inappropriate.

Comment 4

[Jonathan Griffin (:jgriffin)]

Relevant prefs:

marionette.force-local
marionette.defaultPrefs.enabled
marionette.defaultPrefs.port

Comment 5

[Jonathan Griffin (:jgriffin)]

In addition to the above, we should flag any add-ons which attempt to use the following:

"@mozilla.org/marionette;1"
"{786a1369-dca5-4adc-8486-33d23c88010a}"
MarionetteComponent
MarionetteServer

Comment 6

[Jonathan Griffin (:jgriffin)]

who is the right person to implement AMO checks for these things?  I'd be happy to help but have no idea where to start.

Comment 7

[Daniel Veditz [:dveditz]]

Jorge: are you still maintaining the AMO validator? If not please point us in the right direction.

Shouldn't this bug be moved to the addons.mozilla.org product? I was thinking this was the security-review bug but that ended up being tracked in bug 870576; this is now a "let AMO reviewers know what they need to watch for" bug.

mdas is probably not the right assignee anymore.

Comment 8

[Jorge Villalobos [:jorgev] (he/him)]

We already flag non-extension preferences, so comment #4 shouldn't be necessary. Flagging what's in comment #5 is what matters in this case.

When do we need this by?

Comment 9

[Jonathan Griffin (:jgriffin)]

This blocks landing of bug 870445, which dmose's team is investigating for use in testing Talkilla, so I think the answer is "as soon as possible".

Comment 10

[Kris Maglione [:kmag]]

We should add specific checks for the preferences in comment 4. We only flag non-extension preferences in defaults/preferences/*.js, and in any case those preferences should warrant the same kind of special treatment we give network.* preferences.

Comment 11

[Dan Mosedale (:dmosedale, :dmose)]

So we (the Talkilla team) has decided that for reasons of code maturity, we need to go with a Selenium fork for the moment, but Marionette is clearly the architecturally sane path forward in the slightly longer term.

This means that it's not quite so urgent for us, but it'll be exciting to see this ride the trains into Firefox proper, as that should help it mature quickly and improve Firefox's standing as a development environment significantly.

Comment 12

[Jonathan Griffin (:jgriffin)]

Hey Matt, any update on this?  Is there anything I can do to help speed this along?  We are trying to land bug 870445 ahead of the Selenium Conference on June 10, and this is the only open blocker for that.

Comment 13

[David Burns :automatedtester]

PR 204 created https://github.com/mozilla/amo-validator/pull/204

Comment 14

[Jonathan Griffin (:jgriffin)]

Merged as https://github.com/mozilla/amo-validator/commit/4a40a7cd179c4d4eaa0a23d7ff9f8969456adfa2