Add-ons/Reviewers/Guide/Reviewing: Difference between revisions

From MozillaWiki
< Add-ons‎ | Reviewers‎ | Guide
Jump to navigation Jump to search
(Updated to reflect accurate reviewer actions for policy violations)
 
(46 intermediate revisions by 7 users not shown)
Line 1: Line 1:
= Performing a Review =
= Technical 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, good quality, and clearly presented to our users. We also need to make sure developers get quick, clear, and actionable reviews.
== Introduction ==
Add-on reviewers help ensure add-ons are safe to use, reliable and clearly presented to users. We also provide quick, clear, and actionable feedback to developers if issues are found with their add-ons.  


AMO is designed so that popular and well-reviewed add-ons bubble to the top search rankings. The full and preliminary review levels also create two layers that allow us to list add-ons that are not quite ready for general consumption. Since we have good filtering mechanisms on AMO, our general policy is to '''only reject when strictly necessary'''. Rejection is necessary when an add-on has security issues, doesn't meet our content policies, or a few other cases which are spelled out in this guide. If an add-on doesn't meet the criteria for rejection, it should at least get preliminary approval.
All decisions should be based on the official [https://developer.mozilla.org/en-US/Add-ons/AMO/Policy/Reviews Review Policy], please make sure you have read and understood the policy. If you have any questions or need clarifications, the admin team is happy to help. There are no dumb questions when it comes to the review policies!


== Policies and actions ==
The add-on review process consists of the following phases:
The rest of this page explains what our policies and recommended actions are 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, and all issues should be included in the notes sent to developers.
# '''Automatic Review''': When an add-on is uploaded, it undergoes a number of automatic validation steps for the general safety of the add-on.  
# '''Content Review''': Within a fairly short time after submission, add-ons are inspected by a human to ensure that the listing adheres to content review guidelines. This includes metadata such as the add-on name and description.
# '''Technical Code Review''': The source code of the add-on is inspected to ensure it is in compliance with our review policies.
# '''Basic Functionality Testing''': Once the source code is verified safe, the add-on must be given a basic test in functionality to ensure that it acts as described.


= Step 1: Review Add-on Metadata  =
In the following sections, you will learn more about each phase of review and how to go about it. Before we go into detail about the phases, we’d like to tell you a few helpful tips on how to get your system ready for reviewing.


Before getting started, here's a very important legal note: reviews '''must not''' involve checking for copyright or trademark violations. You should not take '''any''' action on an add-on because you suspect it copies code from others without permission or may otherwise infringe somebody else's copyright or trademark. The DMCA is a law that gives us legal protection from being held responsible for copyright infringement by users who post content to our site, but only if our conduct qualifies us for such protection and we follow exactly the procedures laid out in the DMCA. Determining copyright or trademark infringement is complicated and you will not have enough information to make those determinations.
== Setting up a Review Environment ==
You can complete most of the review using the reviewer tools at addons.mozilla.org. A file viewer is available that will also diff between add-on versions, and the validation report shows additional information that may be helpful during the review. The validation results are also shown inline in the file viewer.


If you have any concerns about the legality or legitimacy of an add-on, please email amo-editors AT mozilla DOT org.
Most of the reviewer tools pages are fairly straightforward, but we have [https://youtu.be/0yLfVcuXb04 prepared a video] that will give you an introduction to the tools. We recommend taking a moment to watch it now.


== Policies and Actions  ==
=== Security Considerations ===
As a reviewer, you have access to systems that allow approving and rejecting large amounts of add-ons. Add-ons that have not been reviewed may contain code that aims to control your computer or steal personal information, such as login tokens to addons.mozilla.org.


{| cellspacing="0" cellpadding="1" border="0" style="width: 80%"
As such, you need to be conscious and security-aware when accessing and using the reviewer tools. Please adhere to the following security guidelines:
|+
|-
! style="border-bottom: 2px solid black" scope="col" | Issue
! style="border-bottom: 2px solid black" scope="col" | Action
! style="border-bottom: 2px solid black" scope="col" | Notes
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | The add-on doesn't meet AMO content policies.
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Reject
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | These are add-ons that embed known spyware or malware,
illegal and criminal add-ons, such as click fraud generators, warez download and directory assistants, child pornography finders, etc., and
add-ons whose main purpose is to facilitate access to pornographic or copyrighted material.
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | The add-on uses Mozilla trademarks in its name.
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Reject
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | We allow it when used at the end of the name in this way: "Video downloader for Firefox", "Inspector for Mozilla". Not allowed: "Firefox downloader", "My Mozilla downloader", etc.
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | The add-on doesn't provide enough information in its descriptions for users to figure out what it does.
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Request more information
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" |
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | The add-on name and/or code appear copied or very similar to a popular add-on (like AdBlock Plus).
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Request super-review
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | These add-ons can include malicious code and trick users into thinking they are the original one. We accept add-on forks or with similar features, but we don't want to confuse users.
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" |
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Other copyright suspicions
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | No action
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | We have a legal obligation ''not'' to take action. See note at the beginning of this section.
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Missing Privacy Policy when necessary
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Preliminary Review
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | 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.
|}


== Add-on Relevance ==
* When accessing the reviewer tools, never do so in the same profile that you are testing an add-on with. It is recommended to use a separate profile only for accessing the reviewer tools that has third party add-ons disabled.
* Use a separate profile for testing add-ons, which you can regularly throw away and recreate.  
* When testing add-ons that require additional software (e.g. through native messaging), always make use of a VM to facilitate testing. You can use [https://www.virtualbox.org/ VirtualBox] along with a free image from [https://developer.microsoft.com/en-us/microsoft-edge/tools/vms/ modern.ie]. Make use of snapshots to reset the VM after installing third party software.
* Never log in to the reviewer tools from within the VM where you install third party software related to add-ons, even in a separate profile.


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.
If you become aware of a potential leak or misuse of your AMO credentials, please get in touch with the admin team at amo-admins [at] mozilla [dot] com immediately so we can analyze ensure your account was not accessed by a malicious third party.


In some cases, however, reviewers should deny full review to add-ons due to their usefulness. Add-ons that commonly fall into this category are:
== Automatic Review and Risk Categorization ==
* Add-ons that help access very specific webpages. For example, an add-on that makes it easier to use an internal business site or a university library.
When a new add-on version is uploaded to addons.mozilla.org (AMO), the [https://github.com/mozilla/addons-linter/ addons-linter] runs a set of validation checks for potential issues. Following this validation, add-ons are signed and listed on AMO immediately. They are then shown in the list of auto-approved add-ons in the reviewer tools.
* Add-ons targeted to a limited geographical area. For example, an add-on for a local city newspaper or radio station.  
* Add-ons that don't do more than link to websites through buttons or menu items.
* YouTube (or other) video downloaders. We already have too many add-ons of this kind, with little to no distinction between them. Unless the add-on provides something really innovative to video downloading, it should not get full approval.


When in doubt, don't apply this policy.
[[File:Review Queue.png|none]]


= Step 2: Automatic validation =
As you can see from the screenshot above, the review queue is sorted according to weight, otherwise known as risk. The highest weighted add-ons are sorted at the top of the list.  
A number of factors are included in the risk calculation, including abuse reports, therefore add-ons that turn out to be malicious will quickly bubble up in the list.


We have a 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.
While there are a lot of add-ons in the auto-approved list, we ask you to focus on those with the highest calculated risk. These are the most important add-ons to review.


[[Image:Validation-link.png|center|Add-on validation link]]
== Content Review ==
When an add-on is submitted and passes automatic review, a team of content reviewers take a look to ensure the add-on listing is in line with our content policies. This is useful for example to filter out spam submissions or obvious malicious content.


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.
As a technical reviewer, you must be familiar with the content review guidelines and be ready to enforce them. Add-ons may be rejected in content review for the following reasons:


If the validator shows a validation error and no results, it's possible that reloading the validation page will fix the problem. If the problem persists contact the review team on the #amo-editors IRC channel or pick a different add-on to review.
* Obscene or pornographic images in the icon or screenshots
* Hate speech in the add-on listing (note: anything promoting Nazism or that uses Nazi symbols must be rejected) 
* The add-on is spam
* The add-on infringes on Mozilla copyright or trademarks.  


==== Policies and actions  ====
Content reviewers make decisions solely based on information provided in the add-on listing. It is possible that an add-on’s metadata could be considered acceptable but the code is unacceptable, and vice versa.


{| width="80%" cellspacing="0" cellpadding="1" border="0"
Confirming approval as a technical reviewer also marks the add-on as approved for content review. If you see an add-on that has not been content reviewed, please undergo a full content review of the metadata.
|-
! style="border-bottom: 2px solid black" scope="col" | Issue
! style="border-bottom: 2px solid black" scope="col" | Action
! style="border-bottom: 2px solid black" scope="col" | Notes
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Using eval, Function(), setTimeout, setInterval to evaluate JS code.
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Reject
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | eval can be allowed when it is used to patch Firefox functions with local code. setTimeout and setInterval can be used with hardcoded JS strings, but using closures is preferred.
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Remote script injection
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Reject
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | 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.
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | <browser> or <iframe> elements with no type
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Reject
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | See the [https://developer.mozilla.org/en/XUL/iframe#a-browser.type iframe documentation]. The type needs to be set to "content" (or "content-targetable", or "content-primary") ''before'' anything is loaded on that iframe. If the frame or browser is used to load chrome content, it's fine not to use a type as long as that's clear from reading the source code.
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Inserting remote content with innerHTML
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Reject
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | The canned response points to the [https://developer.mozilla.org/en-US/Add-ons/Overlay_Extensions/XUL_School/DOM_Building_and_HTML_Insertion right documentation about this]. innerHTML will execute any JS code contained in the injected string, so it needs to be very clear that there is no executable code in it. The docs offer various ways to ensure this.
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Using DOM Mutation events
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Preliminary Review
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Just add a note if this is a first warning and the add-on has been approved with this code before. [https://developer.mozilla.org/en-US/docs/Web/API/MutationObserver Mutation Observers] are the recommended alternative.
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Native object prototype extension / Using Prototype library
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Preliminary Review
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | This only applies in XUL overlays, where the prototype extension affects the prototypes used by Firefox code and other overlays.
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Storing passwords or other sensitive user data in the preferences
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Preliminary Review
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" |
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Changing Firefox preferences without user consent
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Preliminary Review
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | These include: network preferences, update system preferences, homepage, User Agent string.
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Changing security preferences, permissions, certificates (nsIX509CertDB)
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Request super-review
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" |
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Using nsIProcess
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Request super-review
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" |
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Using JS c-types
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Request super-review
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" |
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Localization errors
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Ignore
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" |
|}


There are many other validation flags of varying importance. If you're unsure about which action to take, please ask on the mailing list.
Now is a good time to take a minute and read the [[Add-ons/Reviewers/Content_Review_Guidelines|content review guidelines]] and the linked [https://www.mozilla.org/about/legal/acceptable-use/ Mozilla Acceptable Use policy] to make sure you have understood them.


=== Step 3: Code Review ===
== Technical Code Review ==
This is where you will be spending the most amount of time. With help of the review tools, you will be able to determine if the code provided by the developer complies to our [https://developer.mozilla.org/en-US/Add-ons/AMO/Policy/Reviews policies]. Along with actually viewing the code, you should make yourself familiar with the review history to ensure consistent replies.


Every line of add-on code must be reviewed. The code validator can't detect all possible security or code quality issues, so we must always be on the lookout for bad code. For versions in the Preliminary Review queue, the code review should just ensure the add-on's safety.  
In this section we will introduce you to the review history, explain how to go about the code review, and end with a few tips and tricks to help you in reviewing. To get started, click on an extension in the queue to be taken to its review page.


All review pages have a View Contents link that take you to the code browser page. Pending updates also have a Compare with Public Version link next to it, which will show you the code with the changed sections highlighted.
=== Review History ===
When looking at the review page, you will see information on the latest add-on version that was submitted. It may contain notes from the developer in the “Notes to Reviewers” section. There may also be past conversation between reviewers and developers that you should read.


For updates, the compare link should be used. However, it can sometimes fail to work for very large add-ons. In that case, you can either review the full source code, or download the new and public files to your system and compare them using tools like [http://code.google.com/p/amo-editor-tools/source/browse/trunk/sh/xpidiff.sh xpidiff.sh] or [https://wiki.mozilla.org/AMO:Editors/WinMerge WinMerge].
[[File:Review History.png|none]]


Validation results are integrated into the code viewer, so you can see validation warnings in context. For Add-ons SDK extensions, known library files are shaded in grey, and their origin is stated in the popup that appears when hovering over them.
Unless an add-on was just recently submitted, it will have past versions that will also contain notes. You do not need to read through each and every version’s notes, but it is helpful to read up on the notes from the last rejection or confirmed approval to ensure all issues are taken care of.


==== Policies and Actions  ====
Once you have obtained an overview of the review history, you can go on to review the add-on’s code.


{| width="700" cellspacing="0" cellpadding="1" border="0"
=== Code Review ===
|-
Add-ons that have not previously been reviewed by a human need to be inspected in their entirety. Once a manual review has been completed successfully, it is sufficient to review the changes since the last confirmed approval or rejection. If the review version contains a “Compare” link, you can use this to compare against the last confirmed version. Otherwise, use the “Content” link to view the add-on source code.
! style="border-bottom: 2px solid black" scope="col" | Policy
! style="border-bottom: 2px solid black" scope="col" | Action
! style="border-bottom: 2px solid black" scope="col" | Notes
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Remote code download or execution, custom update code
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Reject
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Add-ons should not download remote code in any way. They are free to interact with web APIs as long as all that is being transmitted is data, not code. Add-ons are allowed to insert local scripts into webpages (within reason), but are not allowed to insert remote scripts.
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Modified on unverified Add-ons SDK library code
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Reject
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Library files that are unrecognized will not appear in grey color.
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Using non-release Add-ons SDK library
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Preliminary Review
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | The validator can detect many experimental versions of the SDK, but that doesn't make it OK to use in public add-ons. Public add-ons should use release or beta versions of the SDK. Others should only get preliminary review.
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Using outdated Add-ons SDK library
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Preliminary Review
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Except under special circumstances, we accept the two most recent minor releases of the Add-on SDK. If the validator warns that the add-on is using an outdated SDK version, it is acceptable for full review if it is one release older than the suggested release. Otherwise, it is only eligible for preliminary review.
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | HTTP security violations
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Reject
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | 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.
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Bad or no namespacing
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Preliminary Review
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | 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.
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Preference names without "extensions." prefix
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Preliminary Review (nominations only)
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Add-on preferences should use the "extensions." prefix, and should also have a reasonable namespace. If the add-on is already public, this rule should be ignored.
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Extension preferences names in Sync without "services.sync.extensions." prefix
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Preliminary Review
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Add-on preferences using the Sync service of Firefox should use the "services.sync.extensions." prefix, and should also have a reasonable namespace.
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Privacy issues
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Preliminary Review
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | 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.
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Performance problems
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Preliminary Review
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Synchronous requests, noticeably inefficient code, random UI freezes, loading large amounts of JS code (in the order of tens of thousands of lines of code) directly in overlays.
|- style="vertical-align: top;"
| style="padding: 0.5ex 1ex 1ex 0pt; border-bottom: 1px solid black;" | Not using prefwindow for preferences
| style="padding: 0.5ex 1ex 1ex 0pt; border-bottom: 1px solid black;" | Add Note
| style="padding: 0.5ex 1ex 1ex 0pt; border-bottom: 1px solid black;" | If an add-on has a preferences window, we recommend that authors do 2 things: 1) use the ''prefwindow'' element, 2) add the line in install.rdf that enables this window to be opened from the Add-ons Manager window.
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Duplicate / hidden files or folders
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Add Note
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | See canned response.
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Unpacked add-on writes files in own installation directory
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Preliminary Review
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Writing in the installation directory can generate problems with the Add-ons Manager, where the add-on can be stuck in an update loop. Add-ons should write all files in the profile dir.
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Bootstrapped Add-on doesn't clean up after itself
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Preliminary Review
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | See section on [[AMO:Editors/EditorGuide/SpecialAddonTypes#Bootstrapped_.28Restartless.29_Add-ons|Bootstrapped Add-on policies]].
|}


The file viewer commonly opens with the manifest.json, which allows you to obtain an overview of the add-on’s functionality. You should now begin to review all the files in the add-on, starting from the first file at the top, subsequently going through each file until the last one is reached.


See the [[AMO:Editors/EditorGuide/AddonReviews/Security|security code review]] page for more detail on the above security rejection reasons.
When finding issues during a review, note them in the review comment field on the review page, preferably using the canned responses. You can read more about how to best make use of the responses and find examples that will help you make better review decisions in our [[Add-ons/Reviewers/Guide/Review Decision|Review Decision Guidelines]].


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.  
It is very important to '''include the file and line number of each issue''' you find. This will help developers quickly identify and address issues. While the issue may seem obvious to you, it may not be readily apparent to the developer. Including line numbers quickly resolves this issue and reduces frustration on all sides.  


By the end of the code review, you'll may have a series of notes that will require the add-on to be significantly rewritten. If you think that's the case, it's OK to resolve the review without proceeding to the next step. If the add-on needs work but is safe to use, it can be given a preliminary review approval without performing any testing. If the add-on needs work and you are not sure if it is safe to use, then leave the add-on for someone else to review, request a admin review, or ask for help in the mailing list.
If the same issue occurs more than a few times in each file, you should cite at least three lines where the issue occurs. If there are more than three occurrences of the same issue, you can let the developer know that similar issues exist in other parts of the file.


=== Step 4: Feature Review  ===
=== Libraries and other Minified Code ===
It's very common for add-ons to use libraries or frameworks such as jQuery or React. Some libraries however, were written with only websites in mind, where security boundaries are very strict. Therefore, we have to review libraries, keeping in mind they are being used in a more privileged context.


The last step in a review is to install and test the add-on.  
Our validation checks will attempt to detect known third-party libraries, but won't work in every case. If detected, the library’s code won't generate warnings and it will be greyed out in the code viewer and does not have to be reviewed.


==== Testing setup  ====
Other libraries have to be reviewed in a similar way that normal add-on code is reviewed. Please go through the library code and make sure it complies to our policies. If you come across library methods that are potentially unsafe, find out if the method is actually used in the add-on. For example, jQuery uses the $.parseHTML() function which has security considerations, but we still allow the use of jQuery if the add-on does not use these functions.


These are a few settings and tools you should use or consider using when setting up your add-on testing environment:
The developer must provide links to the library files they used in accordance with our [https://developer.mozilla.org/en-US/Add-ons/Third_Party_Library_Usage library usage guidelines]. If you come across a minified library that is not known to our system, please take the following steps:
# Check if the developer has provided a link to the exact file being used, as per our library usage guidelines.
# If the links are not available, please request more information using the reviewer reply feature.
# Verify the link is from the original maintainer’s website
# Verify the file included in the add-on is an exact match to the linked original file on the maintainer’s website.
# Search for the corresponding unminified file on the maintainer’s website
# Review the unminified file as described above


*'''Always use a separate profile for testing''', never your main profile. See [https://developer.mozilla.org/en/Setting_up_extension_development_environment Setting up an extension development environment].
If the links are not available or do not point to the original maintainer’s website, please request more information from the developer using the canned response. If the file contents do not match, you can reject the add-on. Canned responses are available for this purpose. If you believe a library is used sufficiently often that it should be added to our automatic checks, please get in touch with the admin team.
*Ideally you should perform your tests in a virtual machine. It is always useful in case you need to test in multiple platforms. [http://www.virtualbox.org/ VirtualBox] is free and supports most platforms.
*Enable full Error Console reporting, as described in [https://developer.mozilla.org/en/Setting_up_extension_development_environment#Development_preferences the Development Preferences section].
*These add-ons help you inspect add-on behavior and find possible solutions for problems:
**[https://addons.mozilla.org/en-US/firefox/addon/6622 DOM Inspector]. Analyze XUL and HTML layout, CSS and even JS objects.
**[https://addons.mozilla.org/en-US/firefox/addon/1815 Console<sup>2</sup>] for detailed Console logging.  
**[https://addons.mozilla.org/en-US/firefox/addon/966 Tamper Data] or [https://addons.mozilla.org/en-US/firefox/addon/3829 Live HTTP Headers] for HTTP traffic analysis.
**[https://addons.mozilla.org/en-US/firefox/addon/2490 Leak Monitor] to detect some types of memory leak. See [https://wiki.mozilla.org/MozillaQualityAssurance:Home_Page:Firefox_3.0_TestPlan:Leaks:LeakTesting-How-To#Leak_Gauge Leak Gauge] for a more general solution.
**[https://addons.mozilla.org/en-US/firefox/addon/extension-test Extension Test] to detect loose variables and DOM IDs, prototype extension, dangerous category registration, and other difficult to spot problems. Also automatically sets the required dom.report_all_js_exceptions and javascript.options.showInConsole preferences.
*To test Fennec add-ons, you'll need to [https://wiki.mozilla.org/Fennec#Test_Builds install Fennec]. It is preferred that you test in a supported mobile device. If a Fennec nomination has been waiting for long, it's OK to test with the desktop XULRunner application.
*For online malware scanning, you can use [http://www.virustotal.com/ Virus Total], [http://www.kaspersky.com/scanforvirus Kaspersky online scan] and [http://virusscan.jotti.org Jotti online scan]. AMO performs virus checks, and binary add-ons should be admin-reviewed anyway, but if you suspect anything, those are good tools to use.


==== Installing and testing  ====
Aside from libraries, many add-ons include minified, obfuscated or otherwise machine-generated 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, use the canned info request about minified code. You can read more about our [https://developer.mozilla.org/en-US/Add-ons/Source_Code_Submission Source Code Submission guidelines].


Add-ons are normally cross-platform, so there will only be a single file to review, in the same section where the validator and source links are located. If the add-on is offered for a limited number of platforms, there will be individual links for each one of them. In this case all supported platforms should be tested.  
=== Tips & Tricks ===
Reviewing add-ons is a lot about following data around through the security boundaries within the add-on. A web page has less privileges than a WebExtension content page, which has less privileges than the WebExtension background page.  


Regarding applications, you don't need to test the add-on for all applications it supports. If the add-on supports Firefox and others, it's OK to only test on Firefox. If, however, an add-on update introduces Fennec or other application support, the add-on should be tested on it. The applications we support for reviews are listed on [https://addons.mozilla.org/en-US/firefox/pages/appversions this page].
We recommend that you concentrate on finding code where data is being injected or executed (e.g. use of innerHTML), then backtracking to see where the data originates to determine if it is safe.  


==== Policies and actions  ====
Likewise, when data leaves the user’s computer, you’d want to backtrack to the origin to identify what exact data is being transmitted.


{| width="700" cellspacing="0" cellpadding="1" border="0"
If you stop at each function that has a potentially malicious nature and then track forward to see where it is used, you will end up reviewing many files more than once. This approach takes substantially more time, and we do not recommend it.
|-
! style="border-bottom: 2px solid black" scope="col" | Policy
! style="border-bottom: 2px solid black" scope="col" | Action
! style="border-bottom: 2px solid black" scope="col" | Notes
|- style="vertical-align: top;" id="security-violations"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Security violations
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Reject
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Adding HTTP content to secure pages. Visit HTTPS sites like addons.mozilla.org and make sure the identity button is unchanged. This is specially important for add-ons that insert scripts into sites.
[[Image:Identity-button.png|center|Identity button]]


|- style="vertical-align: top;" id="no-surprises-violations"
== Basic Functionality Testing ==
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | [https://addons.mozilla.org/en-US/developers/docs/policies/reviews#section-defaults No Surprises] violation
The last step in a review is to install and test the add-on. You can do so by navigating to `about:debugging` in a separate profile and installing the add-on as a temporary add-on. If you have downloaded and extracted the add-on you can also make use of ''[https://github.com/mozilla/web-ext web-ext run]'' to start a browser with the add-on installed.
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Preliminary Review
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Changing homepage, default search provider, including unexpected ads or content changes without explicit user opt-in.
|- style="vertical-align: top;" id="privacy-violations"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Privacy violations
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Preliminary Review
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Incorrect or insufficient privacy policies, not respecting Private Mode.
|- style="vertical-align: top;" id="modal-startup"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Showing a modal dialog at startup
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Preliminary Review
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Many add-ons open dialogs or new tabs at startup, mostly offering information on getting started. This is useful, but it shouldn't block the user from using the browser. Opening modal (blocking) dialogs at startup is not allowed. Non-modal dialogs, separate windows or new tabs are allowed.
|- style="vertical-align: top;" id="errors"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Errors in the Error Console
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Preliminary Review
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" |
|- style="vertical-align: top;" id="confusing"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Add-on is very hard to use without instructions
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Preliminary Review
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | If the add-on is difficult to use, there should be instructions included in the add-on descriptions, or in a startup page or window.
|- style="vertical-align: top;" id="sticky-toolbar-buttons"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Toolbar buttons are not customizable
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Preliminary Review
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" |
|- style="vertical-align: top;" id="leaks"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Memory leaks from content or chrome
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Preliminary Review
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | If an add-on touches the content in any way, we need to test the add-on on a page where it works, close the page, and then look in about:memory?verbose. Remaining compartments related to the page indicate a memory leak. After disabling the add-on (and minimizing memory usage) the add-on should no longer use any memory. If it does, it leaks to chrome. More details [https://developer.mozilla.org/en/Extensions/Performance_best_practices_in_extensions#Avoid_Creating_Memory_Leaks here] and
[https://developer.mozilla.org/en/Zombie_Compartments#Proactive_checking_of_add-ons here].
|- style="vertical-align: top;" id="affiliate"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Affiliate linking
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Preliminary Review
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | See details below.
|- style="vertical-align: top;" id="third-party-software"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Requires third party software or paid registration
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Admin Review
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | This excludes add-ons that require other add-ons to function, like Firebug extensions.
|- style="vertical-align: top;" id="content-ads"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Inserts ads into content
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Admin Review
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | The rules in these cases are complex. They need to be clearly labeled as coming from the add-on (otherwise Prelim). They can't remove or replace existing ads (otherwise Reject). They need to follow No Surprises (otherwise Prelim). And there are also security concerns (Reject) and privacy concerns (Prelim).
|}


Other tests to perform:  
For the rare circumstance that you need to test an add-on persisting over a restart, you will need to use [https://www.mozilla.org/en-US/firefox/channel/desktop/ Firefox Nightly] with [[Add-ons/Extension_Signing#FAQ|signing disabled]] or an [[Add-ons/Extension_Signing#Unbranded_Builds|unbranded build]] to test.


*Visit a very simple website like example.org and inspect its DOM, looking for any changes. Again, this is particularly important for extensions that insert scripts or make DOM changes.
You are not expected to extensively test each feature of the add-on. Instead, expect to spend not more than 10 minutes reviewing the add-on’s functionality to see if it behaves as described. You can use the testing to answer the following questions:
*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).
*Test all add-on features, within reason. If there 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 to not cause errors to appear in the Error Console.
*Open the Customize Toolbar dialog and make sure that all buttons have appropriate icons and label text.
*Affiliate linking. 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).


=== Step 5: Resolution  ===
* '''Does the add-on act as described by the summary and description?'''<br />If it says it is an ad-blocker, does it do that or does it instead change your search engine and take over your new tab page?


Choose the appropriate resolution and include all of your notes. Make sure you use a corteous and professional tone, and be as helpful as you can when pointing out problems or areas for improvement. If you are pushing the add-on public, thank the author for the time and effort they have put in. Once you're ready, click the ''Process Action'' button.  
* '''Is the add-on generally functional?'''<br />If you click on the browser action, page action, sidebar, or options page, do the pages load correctly? We don’t reject for bugs or incomplete features, but if the add-on just does not work at all, we’d like to protect from a bad user experience.


The submission of the review form can fail for a number of reasons, so it is highly recommended that you copy of your notes to the clipboard before you click on the submit button. You can also use add-ons like [https://addons.mozilla.org/en-US/firefox/addon/5761 Textarea Cache] for this.  
* '''Does logging in using developer-provided credentials give you access to the described functionality?'''<br />Developers are required to provide testing credentials if they use a service. Use these credentials to log in, and ensure that the advertised core functionality exists.


'''Editor tour:''' remember to ask the admin editor to review your response before sending it.  
* '''If opt-in/opt-out UI is for data collection presented, does it convey the consequences correctly? Does declining actually inhibit the data collection?'''<br />While you could see this during code review, it is sometimes easier to test. Try to trigger the opt-in/opt-out mechanism, and check if the wording informs the user about the consequences.<br /><br />An opt-in needs to have the default action be to decline, and at the same time it needs to remain possible to use the core functionality of the add-on after the user declines.<br /><br />Also check the Network panel in the Developer Tools to make sure the data collection is stopped after declining and does not occur before approving.


[[AMO:Editors/EditorGuide/MailingList|Next: The Mailing List]]
* '''Does the data collected by the add-on comply with our policies?'''<br />The policies have fairly specific provisions on how the developer must handle data collection. With help of the network monitor, you can verify what kind of data is being sent to third party sites. You should load at least one (very simple) web site and identify additional requests by the add-on.
 
* '''Are there any findings from the code review you would like to verify?'''<br />If the code made the impression that sensitive data is being sent but you are unsure, you could use this step to trigger the code in question and check if the data being sent is sensitive.
 
If necessary, you can use the browser console, network monitor and other developer tools to help with functional testing.
 
== Completing the Review ==
Add-ons must be reviewed '''completely''', noting issues in the review comments field as you find them. We want to avoid sending multiple partial reviews to developers and give them an opportunity to address all policy violations at once in their next submission.
 
Depending on the severity of the issue, add-on versions can either be rejected immediately or delayed rejected with the policy violations found. If any additional non-policy violation details need to be communicated, it can be done using a reviewer reply.
 
In order to support you in communicating with developer, and making consistent review decisions, we have prepared a set of [[Add-ons/Reviewers/Guide/Review Decision|Review Decision Guidelines]] that you should take a moment to read right now. It contains important information on when a rejection is warranted and will explain how to complete the review.
 
== Conclusion ==
After reading this document you should be all set up to begin reviewing. You now know how to set up a review environment, are well versed in the different phases from add-on submission to completed review, and have gathered knowledge on how to make a well-informed decision on the review outcome.
 
If you have any feedback on the review process or there is anything unclear in this guide, please get in touch with the admin team. Your comments will be valuable to improve our documentation and make sure you have all the tools you need to complete reviews effectively.
 
[[Add-ons/Reviewers/Guide/Tools|Previous: Tools]] [[Add-ons/Reviewers/Guide/Review_Decision|Next: Review Decisions]]

Latest revision as of 16:23, 28 June 2024

Technical Review

Introduction

Add-on reviewers help ensure add-ons are safe to use, reliable and clearly presented to users. We also provide quick, clear, and actionable feedback to developers if issues are found with their add-ons.

All decisions should be based on the official Review Policy, please make sure you have read and understood the policy. If you have any questions or need clarifications, the admin team is happy to help. There are no dumb questions when it comes to the review policies!

The add-on review process consists of the following phases:

  1. Automatic Review: When an add-on is uploaded, it undergoes a number of automatic validation steps for the general safety of the add-on.
  2. Content Review: Within a fairly short time after submission, add-ons are inspected by a human to ensure that the listing adheres to content review guidelines. This includes metadata such as the add-on name and description.
  3. Technical Code Review: The source code of the add-on is inspected to ensure it is in compliance with our review policies.
  4. Basic Functionality Testing: Once the source code is verified safe, the add-on must be given a basic test in functionality to ensure that it acts as described.

In the following sections, you will learn more about each phase of review and how to go about it. Before we go into detail about the phases, we’d like to tell you a few helpful tips on how to get your system ready for reviewing.

Setting up a Review Environment

You can complete most of the review using the reviewer tools at addons.mozilla.org. A file viewer is available that will also diff between add-on versions, and the validation report shows additional information that may be helpful during the review. The validation results are also shown inline in the file viewer.

Most of the reviewer tools pages are fairly straightforward, but we have prepared a video that will give you an introduction to the tools. We recommend taking a moment to watch it now.

Security Considerations

As a reviewer, you have access to systems that allow approving and rejecting large amounts of add-ons. Add-ons that have not been reviewed may contain code that aims to control your computer or steal personal information, such as login tokens to addons.mozilla.org.

As such, you need to be conscious and security-aware when accessing and using the reviewer tools. Please adhere to the following security guidelines:

  • When accessing the reviewer tools, never do so in the same profile that you are testing an add-on with. It is recommended to use a separate profile only for accessing the reviewer tools that has third party add-ons disabled.
  • Use a separate profile for testing add-ons, which you can regularly throw away and recreate.
  • When testing add-ons that require additional software (e.g. through native messaging), always make use of a VM to facilitate testing. You can use VirtualBox along with a free image from modern.ie. Make use of snapshots to reset the VM after installing third party software.
  • Never log in to the reviewer tools from within the VM where you install third party software related to add-ons, even in a separate profile.

If you become aware of a potential leak or misuse of your AMO credentials, please get in touch with the admin team at amo-admins [at] mozilla [dot] com immediately so we can analyze ensure your account was not accessed by a malicious third party.

Automatic Review and Risk Categorization

When a new add-on version is uploaded to addons.mozilla.org (AMO), the addons-linter runs a set of validation checks for potential issues. Following this validation, add-ons are signed and listed on AMO immediately. They are then shown in the list of auto-approved add-ons in the reviewer tools.

Review Queue.png

As you can see from the screenshot above, the review queue is sorted according to weight, otherwise known as risk. The highest weighted add-ons are sorted at the top of the list. A number of factors are included in the risk calculation, including abuse reports, therefore add-ons that turn out to be malicious will quickly bubble up in the list.

While there are a lot of add-ons in the auto-approved list, we ask you to focus on those with the highest calculated risk. These are the most important add-ons to review.

Content Review

When an add-on is submitted and passes automatic review, a team of content reviewers take a look to ensure the add-on listing is in line with our content policies. This is useful for example to filter out spam submissions or obvious malicious content.

As a technical reviewer, you must be familiar with the content review guidelines and be ready to enforce them. Add-ons may be rejected in content review for the following reasons:

  • Obscene or pornographic images in the icon or screenshots
  • Hate speech in the add-on listing (note: anything promoting Nazism or that uses Nazi symbols must be rejected)
  • The add-on is spam
  • The add-on infringes on Mozilla copyright or trademarks.

Content reviewers make decisions solely based on information provided in the add-on listing. It is possible that an add-on’s metadata could be considered acceptable but the code is unacceptable, and vice versa.

Confirming approval as a technical reviewer also marks the add-on as approved for content review. If you see an add-on that has not been content reviewed, please undergo a full content review of the metadata.

Now is a good time to take a minute and read the content review guidelines and the linked Mozilla Acceptable Use policy to make sure you have understood them.

Technical Code Review

This is where you will be spending the most amount of time. With help of the review tools, you will be able to determine if the code provided by the developer complies to our policies. Along with actually viewing the code, you should make yourself familiar with the review history to ensure consistent replies.

In this section we will introduce you to the review history, explain how to go about the code review, and end with a few tips and tricks to help you in reviewing. To get started, click on an extension in the queue to be taken to its review page.

Review History

When looking at the review page, you will see information on the latest add-on version that was submitted. It may contain notes from the developer in the “Notes to Reviewers” section. There may also be past conversation between reviewers and developers that you should read.

Review History.png

Unless an add-on was just recently submitted, it will have past versions that will also contain notes. You do not need to read through each and every version’s notes, but it is helpful to read up on the notes from the last rejection or confirmed approval to ensure all issues are taken care of.

Once you have obtained an overview of the review history, you can go on to review the add-on’s code.

Code Review

Add-ons that have not previously been reviewed by a human need to be inspected in their entirety. Once a manual review has been completed successfully, it is sufficient to review the changes since the last confirmed approval or rejection. If the review version contains a “Compare” link, you can use this to compare against the last confirmed version. Otherwise, use the “Content” link to view the add-on source code.

The file viewer commonly opens with the manifest.json, which allows you to obtain an overview of the add-on’s functionality. You should now begin to review all the files in the add-on, starting from the first file at the top, subsequently going through each file until the last one is reached.

When finding issues during a review, note them in the review comment field on the review page, preferably using the canned responses. You can read more about how to best make use of the responses and find examples that will help you make better review decisions in our Review Decision Guidelines.

It is very important to include the file and line number of each issue you find. This will help developers quickly identify and address issues. While the issue may seem obvious to you, it may not be readily apparent to the developer. Including line numbers quickly resolves this issue and reduces frustration on all sides.

If the same issue occurs more than a few times in each file, you should cite at least three lines where the issue occurs. If there are more than three occurrences of the same issue, you can let the developer know that similar issues exist in other parts of the file.

Libraries and other Minified Code

It's very common for add-ons to use libraries or frameworks such as jQuery or React. Some libraries however, were written with only websites in mind, where security boundaries are very strict. Therefore, we have to review libraries, keeping in mind they are being used in a more privileged context.

Our validation checks will attempt to detect known third-party libraries, but won't work in every case. If detected, the library’s code won't generate warnings and it will be greyed out in the code viewer and does not have to be reviewed.

Other libraries have to be reviewed in a similar way that normal add-on code is reviewed. Please go through the library code and make sure it complies to our policies. If you come across library methods that are potentially unsafe, find out if the method is actually used in the add-on. For example, jQuery uses the $.parseHTML() function which has security considerations, but we still allow the use of jQuery if the add-on does not use these functions.

The developer must provide links to the library files they used in accordance with our library usage guidelines. If you come across a minified library that is not known to our system, please take the following steps:

  1. Check if the developer has provided a link to the exact file being used, as per our library usage guidelines.
  2. If the links are not available, please request more information using the reviewer reply feature.
  3. Verify the link is from the original maintainer’s website
  4. Verify the file included in the add-on is an exact match to the linked original file on the maintainer’s website.
  5. Search for the corresponding unminified file on the maintainer’s website
  6. Review the unminified file as described above

If the links are not available or do not point to the original maintainer’s website, please request more information from the developer using the canned response. If the file contents do not match, you can reject the add-on. Canned responses are available for this purpose. If you believe a library is used sufficiently often that it should be added to our automatic checks, please get in touch with the admin team.

Aside from libraries, many add-ons include minified, obfuscated or otherwise machine-generated 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, use the canned info request about minified code. You can read more about our Source Code Submission guidelines.

Tips & Tricks

Reviewing add-ons is a lot about following data around through the security boundaries within the add-on. A web page has less privileges than a WebExtension content page, which has less privileges than the WebExtension background page.

We recommend that you concentrate on finding code where data is being injected or executed (e.g. use of innerHTML), then backtracking to see where the data originates to determine if it is safe.

Likewise, when data leaves the user’s computer, you’d want to backtrack to the origin to identify what exact data is being transmitted.

If you stop at each function that has a potentially malicious nature and then track forward to see where it is used, you will end up reviewing many files more than once. This approach takes substantially more time, and we do not recommend it.

Basic Functionality Testing

The last step in a review is to install and test the add-on. You can do so by navigating to `about:debugging` in a separate profile and installing the add-on as a temporary add-on. If you have downloaded and extracted the add-on you can also make use of web-ext run to start a browser with the add-on installed.

For the rare circumstance that you need to test an add-on persisting over a restart, you will need to use Firefox Nightly with signing disabled or an unbranded build to test.

You are not expected to extensively test each feature of the add-on. Instead, expect to spend not more than 10 minutes reviewing the add-on’s functionality to see if it behaves as described. You can use the testing to answer the following questions:

  • Does the add-on act as described by the summary and description?
    If it says it is an ad-blocker, does it do that or does it instead change your search engine and take over your new tab page?
  • Is the add-on generally functional?
    If you click on the browser action, page action, sidebar, or options page, do the pages load correctly? We don’t reject for bugs or incomplete features, but if the add-on just does not work at all, we’d like to protect from a bad user experience.
  • Does logging in using developer-provided credentials give you access to the described functionality?
    Developers are required to provide testing credentials if they use a service. Use these credentials to log in, and ensure that the advertised core functionality exists.
  • If opt-in/opt-out UI is for data collection presented, does it convey the consequences correctly? Does declining actually inhibit the data collection?
    While you could see this during code review, it is sometimes easier to test. Try to trigger the opt-in/opt-out mechanism, and check if the wording informs the user about the consequences.

    An opt-in needs to have the default action be to decline, and at the same time it needs to remain possible to use the core functionality of the add-on after the user declines.

    Also check the Network panel in the Developer Tools to make sure the data collection is stopped after declining and does not occur before approving.
  • Does the data collected by the add-on comply with our policies?
    The policies have fairly specific provisions on how the developer must handle data collection. With help of the network monitor, you can verify what kind of data is being sent to third party sites. You should load at least one (very simple) web site and identify additional requests by the add-on.
  • Are there any findings from the code review you would like to verify?
    If the code made the impression that sensitive data is being sent but you are unsure, you could use this step to trigger the code in question and check if the data being sent is sensitive.

If necessary, you can use the browser console, network monitor and other developer tools to help with functional testing.

Completing the Review

Add-ons must be reviewed completely, noting issues in the review comments field as you find them. We want to avoid sending multiple partial reviews to developers and give them an opportunity to address all policy violations at once in their next submission.

Depending on the severity of the issue, add-on versions can either be rejected immediately or delayed rejected with the policy violations found. If any additional non-policy violation details need to be communicated, it can be done using a reviewer reply.

In order to support you in communicating with developer, and making consistent review decisions, we have prepared a set of Review Decision Guidelines that you should take a moment to read right now. It contains important information on when a rejection is warranted and will explain how to complete the review.

Conclusion

After reading this document you should be all set up to begin reviewing. You now know how to set up a review environment, are well versed in the different phases from add-on submission to completed review, and have gathered knowledge on how to make a well-informed decision on the review outcome.

If you have any feedback on the review process or there is anything unclear in this guide, please get in touch with the admin team. Your comments will be valuable to improve our documentation and make sure you have all the tools you need to complete reviews effectively.

Previous: Tools Next: Review Decisions