[IMP] add manifest-external-assets check

This new check verifies that no external sources are referenced on
modules' assets.

Related to: OCA/odoo-pre-commit-hooks#124.

Author: antonag32

README.md

@@ -36,6 +36,7 @@ manifest-author-string | The author key in the manifest file must be a string (w
 manifest-behind-migrations | Manifest version (%s) is lower than migration scripts (%s) | E8145
 manifest-data-duplicated | The file "%s" is duplicated in lines %s from manifest key "%s" | W8125
 manifest-deprecated-key | Deprecated key "%s" in manifest file | C8103
+manifest-external-assets | Assets should be distributed with module's source code. More info at https://httptoolkit.com/blog/public-cdn-risks/ | W8162
 manifest-maintainers-list | The maintainers key in the manifest file must be a list of strings | E8104
 manifest-required-author | One of the following authors must be present in manifest: %s | C8101
 manifest-required-key | Missing required key "%s" in manifest file | C8102
@@ -224,6 +225,11 @@ Checks valid only for odoo <= 13.0
 
     - https://github.com/OCA/pylint-odoo/blob/v9.3.2/testing/resources/test_repo/broken_module/__openerp__.py#L7 Deprecated key "description" in manifest file
 
+ * manifest-external-assets
+
+    - https://github.com/OCA/pylint-odoo/blob/v9.3.2/testing/resources/test_repo/twelve_module/__manifest__.py#L15 Assets should be distributed with module's source code. More info at https://httptoolkit.com/blog/public-cdn-risks/
+    - https://github.com/OCA/pylint-odoo/blob/v9.3.2/testing/resources/test_repo/twelve_module/__manifest__.py#L19 Assets should be distributed with module's source code. More info at https://httptoolkit.com/blog/public-cdn-risks/
+
  * manifest-maintainers-list
 
     - https://github.com/OCA/pylint-odoo/blob/v9.3.2/testing/resources/test_repo/broken_module3/__openerp__.py#L6 The maintainers key in the manifest file must be a list of strings

src/pylint_odoo/checkers/odoo_addons.py

@@ -104,6 +104,7 @@
 import string
 import warnings
 from collections import Counter, defaultdict
+from urllib.parse import urlparse
 
 from astroid import ClassDef, FunctionDef, NodeNG, nodes
 from pylint.checkers import BaseChecker, utils
@@ -269,6 +270,11 @@
         "prefer-env-translation",
         CHECK_DESCRIPTION,
     ),
+    "W8162": (
+        "Asset %s should be distributed with module's source code. More info at https://httptoolkit.com/blog/public-cdn-risks/",
+        "manifest-external-assets",
+        CHECK_DESCRIPTION,
+    ),
 }
 
 DFTL_MANIFEST_REQUIRED_KEYS = ["license"]
@@ -1042,6 +1048,7 @@ def visit_call(self, node):
         "resource-not-exist",
         "website-manifest-key-not-valid-uri",
         "manifest-behind-migrations",
+        "manifest-external-assets",
     )
     def visit_dict(self, node):
         if not os.path.basename(self.linter.current_file) in misc.MANIFEST_FILES or not isinstance(
@@ -1197,6 +1204,32 @@ def visit_dict(self, node):
         ):
             self.add_message("manifest-maintainers-list", node=manifest_keys_nodes.get("maintainers") or node)
 
+        # Check there are no external assets
+        if self.linter.is_message_enabled("manifest-external-assets"):
+            assets_node = None
+            for item in node.items:
+                if item[0].value == "assets":
+                    assets_node = item[1]
+
+            # it is important to use the actual astroid.node instead of manifest_dict, otherwise the
+            # errors are not attributed to the proper node.
+            if assets_node:
+                self._check_manifest_external_assets(assets_node)
+
+    def _check_manifest_external_assets(self, node):
+        def is_external_url(url):
+            return urlparse(url).scheme
+
+        for _, item in node.items:
+            for element in item.elts:
+                if isinstance(element, nodes.Const):
+                    if is_external_url(element.value):
+                        self.add_message("manifest-external-assets", node=element, args=(element.value,))
+                elif isinstance(element, (nodes.Tuple, nodes.List)):
+                    for entry in element.elts:
+                        if isinstance(entry, nodes.Const) and is_external_url(entry.value):
+                            self.add_message("manifest-external-assets", node=element, args=(entry.value,))
+
     def check_deprecated_odoo_method(self, node: NodeNG) -> bool:
         """Verify the given method is not marked as deprecated under the set Odoo versions.
         :param node: Function definition to be checked

testing/resources/test_repo/twelve_module/__manifest__.py

@@ -9,4 +9,15 @@
     'data': [
         'security/ir.model.access.csv',
     ],
+    "assets": {
+        "web.assets_common": [
+            "twelve_module/static/nonexistent.js",
+            "https://shady.cdn.com/somefile.js"
+        ],
+        "web.assets_frontend": [
+            "/twelve_module/hypothetically/good/file.css",
+            ("before", "/web/static/src/css/random.css", "https://bad.idea.com/cool.css"),
+            ["prepend", "/web/static/src/js/hello.js", "http://insecure.and.bad.idea.com/kiwi.js"]
+        ]
+    }
 }

tests/test_main.py

@@ -36,6 +36,7 @@
     "manifest-required-author": 1,
     "manifest-required-key": 1,
     "manifest-version-format": 3,
+    "manifest-external-assets": 3,
     "method-compute": 1,
     "method-inverse": 1,
     "method-required-super": 8,