Errors and warnings from addons-linter

[pdehaan]

I notice we list addons-linter as a devDependency, but don't seem to use it at all: https://github.com/biancadanforth/tracking-protection-shield-study/blob/96be19cdece741a9ae3b769f8868c10c2dd95f74/package.json#L66

So I did a bit of hacking (basically just renaming the XPI to a ZIP) and then ran addons-linter dist/addon.zip, and this is what came out.

Maybe something. Maybe nothing. Maybe we knew everything already and all the stuff has been covered by sec-review and other issues. But it'd be good to at least glance through these and formally sign off on the reported issues and know we're OK with them.

% cp dist/tracking-protection-messaging-study@shield.mozilla.org-1.0.1.xpi dist/addon.zip

% $(npm bin)/addons-linter dist/addon.zip
Validation Summary:

errors          4
notices         1
warnings        6

ERRORS:

Code                 Message                                  Description                                 File                             Line   Column
JSON_DUPLICATE_KEY   Duplicate keys are not allowed in JSON   Duplicate key: Adversal.com found in JSON   lib/disconnect-entitylist.json   46
                     files.
JSON_DUPLICATE_KEY   Duplicate keys are not allowed in JSON   Duplicate key: EFF found in JSON            lib/disconnect-entitylist.json   1825
                     files.
JSON_DUPLICATE_KEY   Duplicate keys are not allowed in JSON   Duplicate key: Etarget found in JSON        lib/disconnect-entitylist.json   1913
                     files.
JSON_DUPLICATE_KEY   Duplicate keys are not allowed in JSON   Duplicate key: Teads.tv found in JSON       lib/disconnect-entitylist.json   3645
                     files.
NOTICES:

Code                    Message                       Description                                                                 File   Line   Column
TYPE_NO_MANIFEST_JSON   manifest.json was not found   No manifest.json was found at the root of the extension. The package file
                                                      must be a ZIP of the extension's files themselves, not of the containing
                                                      directory. See: https://mzl.la/2r2McKv for more on packaging.
WARNINGS:

Code                    Message                                 Description                                                                 File                                   Line   Column
UNSAFE_VAR_ASSIGNMENT   Unsafe assignment to innerHTML          Due to both security and performance concerns, this may not be set using    content/new-tab-variation.js           67     9
                                                                dynamic values which have not been adequately sanitized. This can lead to
                                                                security issues or fairly serious performance degradation.
UNSAFE_VAR_ASSIGNMENT   Unsafe assignment to innerHTML          Due to both security and performance concerns, this may not be set using    content/new-tab-variation.js           74     7
                                                                dynamic values which have not been adequately sanitized. This can lead to
                                                                security issues or fairly serious performance degradation.
UNSAFE_VAR_ASSIGNMENT   Unsafe assignment to innerHTML          Due to both security and performance concerns, this may not be set using    content/scripts/page-action-panel.js   68     5
                                                                dynamic values which have not been adequately sanitized. This can lead to
                                                                security issues or fairly serious performance degradation.
UNSAFE_VAR_ASSIGNMENT   Unsafe assignment to innerHTML          Due to both security and performance concerns, this may not be set using    content/scripts/page-action-panel.js   134    5
                                                                dynamic values which have not been adequately sanitized. This can lead to
                                                                security issues or fairly serious performance degradation.
REMOTE_SCRIPT           Remote scripts are not allowed as per   Please include all scripts in the add-on. For more information, refer to    content/intro-panel.html
                        the Add-on Policies.                    https://mzl.la/2uEOkYp.
REMOTE_SCRIPT           Remote scripts are not allowed as per   Please include all scripts in the add-on. For more information, refer to    content/page-action-panel.html
                        the Add-on Policies.                    https://mzl.la/2uEOkYp.

% echo $? # 1

[biancadanforth]

Thanks Peter!

• Errors (Fixed in next PR)
  • These are from the Disconnect Entitylist. I've confirmed not only the top level key is a duplicate, but the entire entry for all 4 errors are duplicates. I will remove them in the next PR. Also pinging @groovecoder here to let him know, since he helps maintain that list.
• Notices: (Wontfix)
  • Since this is a legacy bootstrapped add-on, there is no manifest.json file. That's for WebExtensions. Our analogous file, if there is one, is install.rdf or chrome.manifest. We can safely ignore that.
• Warnings: (Wontfix)
  • Unsafe variable assignment: I have already discussed these during the Security Review (#25 , #90 ). Because the dynamic data is coming from my JSM and not a user, there should be no risk of XSS attacks.
  • Remote scripts: This method was actually suggested to me by Mossop as a much safer way to use popular third party web development libraries in legacy addon code compared to frame scripts. They are the least privileged kinds of scripts in an add-on. Also I don't even use any libraries/frameworks in this add-on. It's all my own code.

We should stop seeing most of these errors once we move to WEE.