warn on inline script tags in HTML

docs/rules.md

@@ -146,7 +146,6 @@ A :white_check_mark: next to a section of rules means they have all been filed i
 | :negative_squared_cross_mark: | warning | theme_xbl_property | theme | Themes are not allowed to use XBL properties | | | | |
 | :negative_squared_cross_mark: | warning | unsafe_langpack_theme | theme / langpack | Unsafe tag for add-on type | | | | |
 | :x: | warning | remote_src_href | theme / langpack | `src`/`href` attributes must be local | | | | |
-| :white_check_mark: | warning | prefwindow_id | | `<prefwindow>` elements must have IDs | | | | PREFWINDOW_REQUIRES_ID |
 | :x: | warning | iframe_type_unsafe | | iframe/browser missing 'type' attribute | | | | |
 | :x: | warning | iframe_type_unsafe | | Typeless iframes/browsers must be local | | | | |
 | :x: | warning | banned_remote_scripts | | Scripts must not be remote | | | | |
@@ -158,7 +157,7 @@ A :white_check_mark: next to a section of rules means they have all been filed i
 | :x: | warning | extra_closing_tags | | Markup parsing error | | | | |
 | :x: | warning | extra_closing_tags | | Parse error: tag closed before opened | | | | |
 | :x: | warning | invalid_nesting | | Markup invalidly nested | | | | |
-
+| :white_check_mark: | warning | inline script | | Inline script is disallowed by CSP | | | | INLINE_SCRIPT |
 
 ## chrome.manifest
 

src/const.js

@@ -67,10 +67,6 @@ export const RDF_UNALLOWED_TAGS = ['hidden'];
 export const RDF_UNALLOWED_IF_LISTED_TAGS = ['updateKey', 'updateURL'];
 export const RDF_OBSOLETE_TAGS = ['file', 'requires', 'skin'];
 
-export const HTML_TAGS_WITH_REQUIRED_ATTRIBUTES = {
-  prefwindow: ['id'],
-};
-
 // Package type constants.
 export const PACKAGE_ANY = 0;
 export const PACKAGE_EXTENSION = 1;

src/messages/html.js

@@ -1,18 +1,9 @@
 import { gettext as _, singleLineString } from 'utils';
 
-
-export var _tagRequiresAttribute = (tagName, attribute) => {
-  return {
-    code: `${tagName}_REQUIRES_${attribute}`.toUpperCase(),
-    legacyCode: [
-      'markup',
-      'starttag',
-      `${tagName}_${attribute}`,
-    ],
-    message: _(`<${tagName}> missing "${attribute}"`),
-    description: _(singleLineString`The <${tagName}> tag requires the
-      ${attribute}, but it's missing.`),
-  };
+export const INLINE_SCRIPT = {
+  code: 'INLINE_SCRIPT',
+  legacyCode: null,
+  message: _('Inline scripts blocked by default'),
+  description: _(singleLineString`Default CSP rules prevent inline JavaScript
+    from running (https://mzl.la/2pn32nd).`),
 };
-
-export const PREFWINDOW_REQUIRES_ID = _tagRequiresAttribute('prefwindow', 'id');

src/rules/html/ensureRequiredAttributes.js

@@ -1,40 +0,0 @@
-import { HTML_TAGS_WITH_REQUIRED_ATTRIBUTES, VALIDATION_ERROR } from 'const';
-import * as messages from 'messages';
-
-
-export function ensureRequiredAttributes($, filename) {
-  return new Promise((resolve) => {
-    var linterMessages = [];
-
-    for (let tag in HTML_TAGS_WITH_REQUIRED_ATTRIBUTES) {
-      linterMessages = linterMessages.concat(
-        _ensureAttributesInTag($, tag, HTML_TAGS_WITH_REQUIRED_ATTRIBUTES[tag],
-                               filename));
-    }
-
-    resolve(linterMessages);
-  });
-}
-
-export function _ensureAttributesInTag($, tag, attributes, filename) {
-  var linterMessages = [];
-
-  $(tag).each((i, element) => {
-    for (let attributeName of attributes) {
-      var errorCode = `${tag}_REQUIRES_${attributeName}`.toUpperCase();
-
-      if ($(element).attr(attributeName) === undefined) {
-        linterMessages.push({
-          code: errorCode,
-          message: messages[errorCode].message,
-          description: messages[errorCode].description,
-          sourceCode: `<${tag}>`,
-          file: filename,
-          type: VALIDATION_ERROR,
-        });
-      }
-    }
-  });
-
-  return linterMessages;
-}

src/rules/html/index.js

@@ -1 +1 @@
-export * from './ensureRequiredAttributes';
+export * from './warn-on-inline';

src/rules/html/warn-on-inline.js

@@ -0,0 +1,22 @@
+import { VALIDATION_WARNING } from 'const';
+import * as messages from 'messages';
+
+
+export function warnOnInline($, filename) {
+  return new Promise((resolve) => {
+    var linterMessages = [];
+    $('script').each((i, element) => {
+      if ($(element).attr('src') === undefined) {
+        linterMessages.push(
+          Object.assign({}, messages.INLINE_SCRIPT, {
+            /* This could occur in any HTML file, so let's make it
+             * a warning in case they've included any other file.
+             */
+            type: VALIDATION_WARNING,
+            file: filename,
+          }));
+      }
+    });
+    resolve(linterMessages);
+  });
+}

src/scanners/html.js

@@ -11,7 +11,6 @@ export default class HTMLScanner extends BaseScanner {
   _getContents() {
     return new Promise((resolve) => {
       var htmlDoc = cheerio.load(this.contents);
-
       resolve(htmlDoc);
     });
   }

tests/scanners/test.html.js

@@ -1,7 +1,7 @@
 import cheerio from 'cheerio';
 import sinon from 'sinon';
 
-import { VALIDATION_ERROR } from 'const';
+import { VALIDATION_WARNING } from 'const';
 import { getRuleFiles, validHTML } from '../helpers';
 import HTMLScanner from 'scanners/html';
 import * as rules from 'rules/html';
@@ -31,37 +31,30 @@ describe('HTML', function() {
       });
   });
 
-  it('should require <prefwindow> tag to have an id attribute', () => {
-    var badHTML = validHTML(singleLineString`<prefwindow
-      xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"
-      title="My application: configuration"
-      onunload="onUnload(event.target)">
-    </prefwindow>`);
-    var goodHTML = validHTML(singleLineString`<prefwindow
-      xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"
-      id="my-config-dialog"
-      title="My application: configuration"
-      onunload="onUnload(event.target)">
-    </prefwindow>`);
+  it('should require <script> tag to have a src attribute', () => {
+    var badHTML = validHTML('<script>alert()</script>');
     var htmlScanner = new HTMLScanner(badHTML, 'index.html');
 
     return htmlScanner.getContents()
       .then(($) => {
-        return rules.ensureRequiredAttributes($, htmlScanner.filename);
+        return rules.warnOnInline($, htmlScanner.filename);
       })
       .then((linterMessages) => {
         assert.equal(linterMessages.length, 1);
         assert.equal(linterMessages[0].code,
-                     messages.PREFWINDOW_REQUIRES_ID.code);
-        assert.equal(linterMessages[0].sourceCode, '<prefwindow>');
-        assert.equal(linterMessages[0].type, VALIDATION_ERROR);
+                     messages.INLINE_SCRIPT.code);
+        assert.equal(linterMessages[0].type, VALIDATION_WARNING);
+      });
+  });
 
-        // Make sure there are no errors when an ID is provided.
-        htmlScanner = new HTMLScanner(goodHTML, 'index.html');
-        return htmlScanner.getContents();
-      })
+  it('should accept a <script> tag with a src attribute', () => {
+    var goodHTML = validHTML(singleLineString`
+        <script src="">alert()</script>`);
+    var htmlScanner = new HTMLScanner(goodHTML, 'index.html');
+
+    return htmlScanner.getContents()
       .then(($) => {
-        return rules.ensureRequiredAttributes($, htmlScanner.filename);
+        return rules.warnOnInline($, htmlScanner.filename);
       })
       .then((linterMessages) => {
         assert.equal(linterMessages.length, 0);