Add-ons/Reviewers/Guide/Reviewing

From MozillaWiki
< Add-ons‎ | Reviewers‎ | Guide
Jump to: navigation, search

Performing a Review

Reviewer Intro Tour: ask your guide to select an add-on for you to review. Don't submit your first review without their pre-approval!

Add-on reviewers have a big responsibility. We need to ensure add-ons are safe to use, of good quality, and clearly presented to users. We also need to make sure developers get quick, clear, and actionable reviews.

Our general policy is to only reject when necessary. Rejection is necessary when an add-on has security or privacy issues, doesn't meet our content policies, or fits one of the special cases spelled out later in this guide.

Policies and actions

The rest of this page explains policies and recommended actions for common add-on issues. Add-ons must be reviewed completely, and all issues written down as they are found. Once the review is complete, the sum of all noted issues is used to determine what the review resolution should be. Regardless of the result, all issues should be included in the notes sent to developers.

Step 1: Review Add-on Metadata

Our review process does not involve reviewing for copyright infringement of any kind. The DMCA gives us protection from liability for hosting content that infringes copyright because of how hard it would be to review each add-on we host for copyright infringement. If you see something egregious, feel free to escalate it for super-review but we do not specifically review any content for copyright concerns.

If you have any concerns about the legality or legitimacy of an add-on, please email amo-admins AT mozilla DOT org.

Policies and Actions

Issue Action Notes
The add-on doesn't comply with the Mozilla Conditions of Use. Reject These are add-ons which do things like:
  • Anything illegal.
  • Threaten, harass, or violate privacy.
  • Harm users such as by using viruses, malware, or other malicious code.
  • Have the direct purpose of facilitating access to pornography. This includes, add-ons that specifically interact with, or direct users to, porn sites, but does not include add-ons such as image downloaders that may be used to access pornography but also have other purposes.

Make sure to read the rest of the list here.

The add-on improperly uses Mozilla trademarks in its name. Reject We allow the use of certain trademarks when appended to the name of an add-on (e.g., "Video downloader for Firefox", "Inspector for Mozilla") in such a way as not to cause confusion as to the origin of the add-on. We do not allow uses which may suggest that the add-on is a Mozilla product (e.g., "Firefox downloader", "My Mozilla downloader", "Firefox++"). When in doubt, ask an admin or request super-review.
The add-on doesn't provide enough information in its descriptions for users to figure out what it does. Request more information Explain clearly why the provided description is insufficient, and what needs to be done to improve it. Once the developer has corrected the deficiencies and contacted us, the review may be completed without a further upload.
The add-on name and/or code appear copied or very similar to a popular add-on (like AdBlock Plus). Request super-review These add-ons can include malicious code, and trick users into mistaking them for the original. We accept add-on forks, and add-ons with similar features to other add-ons, but we need to be very careful to ensure that they do not mislead users.
Missing Privacy Policy when necessary. Request more information A Privacy Policy is required if an add-on sends any user information to a remote server, even if it is not personally-identifying. Even stats pings require a Privacy Policy. Reject if there's no reply from the developer.

Add-on Relevance

Add-ons must not be rejected because a reviewer doesn't find them useful. We let AMO users make that call, and add-ons that aren't very useful won't gain much usage and have low search rankings, so they'll stay mostly out of the way.

However, if it looks like an add-on shouldn't be listed on AMO, because it's intended for an internal company deployment, for testing purposes, or only private use, it should be rejected with the suggestion to go unlisted instead, and a link to the Distribution page.

Add-ons that are potentially dangerous and have a limited audience (like SSL certificate installers) should be Rejected, while noting to the developer that they can submit the add-on as Unlisted.

Step 2: Automatic validation

We have an extensive set of tests that identify common bad practices and possible security problems with add-on code. Reviewers must run the code validator and inspect the results when performing a review. Each Add-on History entry has a validation link, and you'll want to validate the latest one.

Add-on validation link

Clicking on the link will take you to the validation page, where the automatic code validator will run for that version of the add-on and then the results will be displayed. We recommend opening this link in a new tab.

Policies and actions

Issue Action Notes
Using eval(), Function() to evaluate JS code. Reject eval may sometimes be allowed when it is used carefully to patch Firefox functions with local code.
Using setTimeout(), setInterval(), or properties like onclick to evaluate JS code. Add note They may be used with hardcoded JS strings, but using closures is preferred. Only reject if it's clear from the surrounding code that remote code is being evaluated.
Remote script injection. Reject Add-ons can use data-only APIs, but should never download and execute remote code, not even in the scope of a webpage. Any use of the <script> tag (like createElement("script")) needs to be carefully analyzed. Using remote PAC files is not allowed.
<browser> or

<

iframe> elements with no type attribute, used in privileged documents.

Reject See the iframe documentation. The type must be one of "content", "content-targetable", or "content-primary". This must be done before anything is loaded on that iframe. If the iframe or browser is used to load only chrome content, and it is clear from the code that it will never load anything else, type="chrome" may be used when necessary.
Storing passwords or other sensitive user data in the preferences. Reject Passwords and other sensitive data should be stored in the login service rather than in preferences.
Changing Firefox preferences without user consent. Reject These include: network preferences, update system preferences, homepage, User Agent string. They also must be restored to their previous values when the add-on is uninstalled.
Inserting content with innerHTML. Add note Reject if it's clear from the surrounding code that the code being injected is remote and unsanitized. The canned response points to the preferred documentation about this topic. Assignments to innerHTML will result in the execution of any JavaScript code present in the injected string. Since this issue can often be confusing to developers, make sure to include a reference to a code file and line where this occurs.
Using DOM Mutation events. Add note Mutation Observers are the recommended alternative.
Native object prototype extension / Using the Prototype library. Add note This only applies in XUL overlays, where the prototype extension affects the prototypes used by Firefox code and other overlays.
Changing security preferences, permissions, certificates (nsIX509CertDB). Request super-review
Using nsIProcess. Request super-review
Using JS c-types. Request super-review
Localization errors. Ignore Errors which result in breakage should result in rejection when built-in Firefox UIs are affected, and just a note if only add-on interfaces are affected. Otherwise, they should be ignored.

There are many other validation flags of varying importance. If you're unsure about which action to take, please ask on the mailing list.

Step 3: Code Review

All add-on code must be reviewed. Automation can't detect all possible security or code quality issues, which is why we have human reviewers.

All Add-on History entries have a View Contents link that take you to the code browser page. Updates also have a Compare link, which will show you the code with the changed sections highlighted. For updates, the compare link should be used. Validation results are integrated into the code viewer, so you can see validation warnings in context. It's okay if you prefer to use other tools for code analysis and diffing.

Libraries, frameworks and other unreadable code

It's very common for add-ons to use libraries or frameworks such as jQuery or Bootstrap. Some add-ons use complex frameworks like Kango, usually to achieve cross-browser compatibility. Finally, the Add-ons SDK generates various files around the actual add-on code. Our code validator will try to detect them, but won't work in every case. If detected, the library code won't generate validator warnings and it will be greyed out in the code viewer.

All libraries on this list should be ignored, even if the validator doesn't detect them correctly. All other libraries should be handled carefully. The reviewer should find the original library file and diff it against the one included in the add-on, and also ensure the library doesn't do anything dangerous.

Aside from libraries, many add-ons can include minified, obfuscated or compiled code. Since this code can't be easily reviewed without the original sources, only admin reviewers should review them. The Add-on History entry should indicate if the source code has been provided by the developer. If that's not the case, you can use the canned info request.

Policies and Actions

Issue Action Notes
Remote code download or execution, custom update code. Reject As explained in the validation section, no remote code execution is allowed.
Using plain HTTP for security-sensitive operations. Reject Sending passwords over unprotected HTTP if there's a secure alternative. Sending passwords in the URL or other headers than POSTDATA. Performing any security-sensitive operations over HTTP when there's a secure alternative.
Using non-release version of Add-ons SDK. Reject The validator can detect many experimental versions of the SDK, but that doesn't make it OK to use in add-ons.
Using outdated Add-ons SDK library. Reject We accept the two most recent minor releases of the Add-on SDK.
Bad or no namespacing. Reject All scripts that are included in the main window overlay should have proper namespacing to avoid name conflicts with other add-ons. The name should normally correspond to the add-on name in order to guarantee its uniqueness.
Privacy issues. Reject An add-on can claim to work with a popular website like Twitter, but then send the user data through some other site, most likely owned by the developer. There needs to be a justified reason to handle user data in this manner, and the privacy policy and add-on descriptions need to be very clear about this. Passwords should never be handled in this way, and they should only be transmitted directly to the original API provider (Reject otherwise).
Preference names without "extensions." or "services.sync.extensions." prefix. Add note Add-on preferences should use the "extensions." prefix, and should also have a reasonable namespace (like "extensions.myVideoDownloader.").
Performance problems. Add note Synchronous (non-local) HTTP requests, synchronous SQL queries, noticeably inefficient code, random UI freezes, loading large amounts of JS code directly in overlays. Reject if the issues cause noticeable lag while testing.
Writing files outside of the profile folder. Add note Some add-ons need helper files like SQL DBs or logs. Those files must be written in the profile folder, not the extension installation folder or other locations.
Bootstrapped or SDK add-on doesn't clean up after itself. Add note Some things to look for: the add-on must not require a restart for any of its features to fully work, must not require a restart after being disabled or uninstalled, must unregister all observers and remove all UI when disabled or uninstalled. More details.
Using non-standard preferences UI. Add note If an add-on has a preferences UI, it should use one of the supported methods.
Duplicate / hidden files or folders. Add note See canned response.
New version doesn't follow previous review requests. Reject Ignore if they are minor issues.

In general you should apply your judgement and try to identify code that may appear suspicious or out of place. Try to understand what everything does and how it all fits together.

WebExtensions Policies and Actions

This is a draft of WebExtensions Policies as WebExtensions are relatively new.

Issue Action Notes
Abuse of chrome.storage.sync Reject It's noted in the docs that chrome.storage.sync is for storing preferences and other small amounts of data. It is not provided with any expectations of performance, stability or being able to store large amounts of data.
manifest.json includes permissions "nativeMessaging" Request super-review nativeMessaging must be carefully reviewed by an admin.

Step 4: Feature Review

The last step in a review is to install and test the add-on.

Add-ons are normally cross-platform, in which case there will only be a single XPI to test. If the add-on is offered for a limited number of platforms or has different files for different platforms, there will be individual links for each one in the Add-on History entry.

Regarding application support (Firefox / SeaMonkey / Thunderbird), you don't need to test the add-on on all of them. If the add-on supports Firefox and others, it's OK to only test using Firefox.

Testing setup

Policies and actions

Issue Action Notes
Security violations. Reject Adding HTTP content to HTTPS pages. If the add-on injects content like iframes or images, make sure to visit HTTPS sites the add-on supports and look for any security warnings in the URL bar.
No Surprises violation Reject Changing homepage, default search provider, including unexpected ads or content changes without explicit user opt-in.
Affiliate linking. Reject Some add-ons add affiliate codes to Amazon links (or similar) in order to make money. At the moment we allow this as long as (1) the add-on follows the No Surprises policy, (2) the feature doesn't replace or remove any existing affiliate codes, (3) the affiliate codes aren't inserted in the merchant website's links (inserting Amazon affiliate codes in Amazon.com pages).
Privacy violations. Reject Incorrect or insufficient privacy policies, not respecting Private Browsing Mode.
Errors in the Browser Console. Add note Make sure the errors only occur with the add-on installed and are generated from add-on code and not Firefox code. In the latter case, it should only be noted.
Add-on is very hard to use even with instructions. Add note If the testing instructions for the add-on are missing, use Request more information. We have frequently-used test accounts listed here (ask an admin for the password).
Toolbar buttons are not customizable. Add note It must be possible to remove add-on buttons from the toolbar and move them to the menu panel.
Requires third party software or paid registration. Request super-review
Inserts ads into content. Request super-review The rules in these cases are complex. They need to be clearly labeled as coming from the add-on, can't remove or replace existing ads, and need to follow No Surprises. There are also security concerns and privacy concerns that can lead to rejection.

Other tests to perform:

  • Test all add-on features, within reason. If there are too many, focus on the main features.
  • Remove all added toolbar buttons, disable all added toolbars, and restart the browser. Make sure that buttons and toolbars are all removable and do not reappear on restart. Make sure that missing toolbar buttons do not cause errors to appear in the Error Console.
  • Open the Customize Toolbar UI and make sure that all buttons have appropriate icons and label text.
  • Open the add-on's preferences window, from the Add-ons Manager and elsewhere, and verify that preference changes apply properly. Make sure the window fits all of its contents (a common problem in Mac OS).

Step 5: Resolution

Choose the appropriate resolution and include all of your notes. Make sure you use a courteous and professional tone, and be as helpful as you can when pointing out problems or areas for improvement. The canned responses are very useful in formatting a response. Once you're ready, click the Save button.

Reviewer Intro Tour: remember to ask the guide to give your response a look before sending it. After submitting your first review, make sure you spend some time reading the rest of the guide.

Next: Moderation