Add `is_open_source_project` to metadata validation

Reciprocal licenses can only be used in open source projects.
This change updates the presubmit validation checks to accept an
optional flag `allow_reciprocal_licenses`. When True, the allowlist is
extended to include reciprocal licenses.

Bug: 385020146
Change-Id: I0374658207bc87ffd74e033762ee4973c6e83b3b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6107863
Reviewed-by: Jordan Brown <rop@google.com>
Auto-Submit: Jordan Brown <rop@google.com>
Reviewed-by: Rachael Newitt <renewitt@google.com>
Commit-Queue: Rachael Newitt <renewitt@google.com>

metadata/dependency_metadata.py

@@ -22,6 +22,7 @@ import metadata.fields.custom.version as version_util
 import metadata.fields.known as known_fields
 import metadata.fields.util as util
 import metadata.validation_result as vr
+from metadata.fields.custom.license_allowlist import OPEN_SOURCE_SPDX_LICENSES
 
 
 class DependencyMetadata:
@@ -109,7 +110,31 @@ class DependencyMetadata:
                                field: field_types.MetadataField) -> List[int]:
         return sorted(self._metadata_line_numbers[field])
 
-    def _assess_required_fields(self) -> Set[field_types.MetadataField]:
+    def all_licenses_allowlisted(self, license_field_value: str, is_open_source_project: bool) -> bool:
+        """Returns whether all licenses in the field are allowlisted.
+        Assumes a non-empty license_field_value"""
+        licenses = license_util.process_license_value(
+            license_field_value,
+            atomic_delimiter=known_fields.LICENSE.VALUE_DELIMITER)
+        for lic, valid in licenses:
+            allowed = license_util.is_license_allowlisted(lic, is_open_source_project=is_open_source_project)
+            if not valid or not allowed:
+                return False
+        return True
+
+
+    def only_open_source_licenses(self, license_field_value: str) ->List[str]:
+        """Returns a list of licenses that are only allowed in open source projects."""
+        licenses = license_util.process_license_value(
+            license_field_value,
+            atomic_delimiter=known_fields.LICENSE.VALUE_DELIMITER)
+        open_source_only = []
+        for lic, valid in licenses:
+            if valid and lic in OPEN_SOURCE_SPDX_LICENSES:
+                open_source_only.append(lic)
+        return open_source_only
+
+    def _assess_required_fields(self, is_open_source_project: bool = False) -> Set[field_types.MetadataField]:
         """Returns the set of required fields, based on the current
         metadata.
         """
@@ -127,30 +152,22 @@ class DependencyMetadata:
             # License compatibility with Android must be set if the
             # package is shipped and the license is not in the
             # allowlist.
-            has_allowlisted = False
             license_value = self._metadata.get(known_fields.LICENSE)
-            if license_value:
-                licenses = license_util.process_license_value(
-                    license_value,
-                    atomic_delimiter=known_fields.LICENSE.VALUE_DELIMITER)
-                for _, allowed in licenses:
-                    if allowed:
-                        has_allowlisted = True
-                        break
-
-            if not has_allowlisted:
+            if not license_value or not self.all_licenses_allowlisted(license_value, is_open_source_project):
                 required.add(known_fields.LICENSE_ANDROID_COMPATIBLE)
 
         return required
 
     def validate(self, source_file_dir: str,
-                 repo_root_dir: str) -> List[vr.ValidationResult]:
+                 repo_root_dir: str,
+                 is_open_source_project: bool = False) -> List[vr.ValidationResult]:
         """Validates all the metadata.
 
         Args:
             source_file_dir: the directory of the file that the metadata
                              is from.
             repo_root_dir: the repository's root directory.
+            is_open_source_project: whether the project is open source.
 
         Returns: the metadata's validation results.
         """
@@ -210,7 +227,7 @@ class DependencyMetadata:
                 results.append(field_result)
 
         # Check required fields are present.
-        required_fields = self._assess_required_fields()
+        required_fields = self._assess_required_fields(is_open_source_project=is_open_source_project)
         for field in required_fields:
             if field not in self._metadata:
                 field_name = field.get_name()
@@ -249,6 +266,25 @@ class DependencyMetadata:
                     self.get_field_line_numbers(known_fields.LICENSE_FILE))
                 results.append(result)
 
+
+        if not is_open_source_project:
+            license_value = self._metadata.get(known_fields.LICENSE)
+            if license_value is not None:
+                not_allowed_licenses = self.only_open_source_licenses(license_value)
+                if len(not_allowed_licenses) > 0:
+                    license_result = vr.ValidationWarning(
+                        reason=f"License has a license not in the allowlist."
+                        " (see https://source.chromium.org/chromium/chromium/tools/depot_tools/+/main:metadata/fields/custom/license_allowlist.py).",
+                        additional=[
+                            f"The following license{'s  are' if len(not_allowed_licenses) > 1 else ' is'} only allowed in open source projects: "
+                            f"{util.quoted(not_allowed_licenses)}.",
+                    ])
+
+                    license_result.set_tag(tag="field", value=known_fields.LICENSE.get_name())
+                    license_result.set_lines(
+                        self.get_field_line_numbers(known_fields.LICENSE))
+                    results.append(license_result)
+
         return results
 
     def _return_as_property(self, field: field_types.MetadataField) -> Any:

metadata/fields/custom/license.py

@@ -18,7 +18,7 @@ sys.path.insert(0, _ROOT_DIR)
 import metadata.fields.field_types as field_types
 import metadata.fields.util as util
 import metadata.validation_result as vr
-from metadata.fields.custom.license_allowlist import ALLOWED_LICENSES
+from metadata.fields.custom.license_allowlist import ALLOWED_LICENSES, ALLOWED_OPEN_SOURCE_LICENSES
 
 
 def process_license_value(value: str,
@@ -33,13 +33,13 @@ def process_license_value(value: str,
                           delimiter.
 
     Returns: a list of the constituent licenses within the given value,
-             and whether the constituent license is on the allowlist.
+             and whether the constituent license is a recognized license type.
              e.g. [("Apache, 2.0", True), ("MIT", True),
                    ("custom", False)]
     """
     # Check if the value is on the allowlist as-is, and thus does not
     # require further processing.
-    if is_license_allowlisted(value):
+    if is_license_valid(value):
         return [(value, True)]
 
     breakdown = []
@@ -48,22 +48,33 @@ def process_license_value(value: str,
     for atomic_value in value.split(atomic_delimiter):
         atomic_value = atomic_value.strip()
         breakdown.append(
-            (atomic_value, is_license_allowlisted(atomic_value)))
+            (atomic_value, is_license_valid(
+                atomic_value,
+            ))
+        )
 
     return breakdown
 
 
-def is_license_allowlisted(value: str) -> bool:
+def is_license_valid(value: str) -> bool:
     """Returns whether the value is in the allowlist for license
     types.
     """
-    return value in ALLOWED_LICENSES
+    # The open source allowlist is the most permissive.
+    return value in ALLOWED_OPEN_SOURCE_LICENSES
 
+def is_license_allowlisted(value: str, is_open_source_project: bool = False) -> bool:
+    """Returns whether the value is in the allowlist for license
+    types.
+    """
+    if is_open_source_project:
+        return value in ALLOWED_OPEN_SOURCE_LICENSES
+    return value in ALLOWED_LICENSES
 
 class LicenseField(field_types.SingleLineTextField):
   """Custom field for the package's license type(s).
 
-    e.g. Apache 2.0, MIT, BSD, Public Domain.
+    e.g. Apache-2.0, MIT, BSD-2.0
     """
   def __init__(self):
     super().__init__(name="License")
@@ -75,7 +86,8 @@ class LicenseField(field_types.SingleLineTextField):
         """
     not_allowlisted = []
     licenses = process_license_value(value,
-                                         atomic_delimiter=self.VALUE_DELIMITER)
+          atomic_delimiter=self.VALUE_DELIMITER,
+    )
     for license, allowed in licenses:
       if util.is_empty(license):
         return vr.ValidationError(

metadata/fields/custom/license_allowlist.py

@@ -60,10 +60,6 @@ ALLOWED_SPDX_LICENSES = frozenset([
     "X11",
     "Zlib",
     "libtiff",
-    # reciprocal. TODO(b/385020146): Only allow for opensource projects.
-    "APSL-2.0",
-    "MPL-1.1",
-    "MPL-2.0",
 ])
 
 # These are licenses that are not in the SPDX license list, but are identified
@@ -110,7 +106,7 @@ EXTENDED_LICENSE_CLASSIFIERS = frozenset([
 ])
 
 # These licenses are only allowed in open source projects due to their
-# reciprocal requirements. TODO(b/385020146): Enforce this restriction.
+# reciprocal requirements.
 OPEN_SOURCE_SPDX_LICENSES = frozenset([
     # reciprocal.
     "APSL-2.0",

metadata/tests/data/README.chromium.test.multi-invalid

@@ -4,7 +4,7 @@ URL: https://www.example.com/metadata,
      https://www.example.com/parser
 Version: 1.0.12
 Date: 2020-12-03
-License: Apache-2.0 and MIT and not_an_spdx
+License: Apache-2.0, MIT
 License File: LICENSE
 Security Critical: yes
 Shipped: yes
@@ -21,7 +21,7 @@ EXCEPT:
 
 -------------------- DEPENDENCY DIVIDER --------------------
 
-Name: Test-B README for Chromium metadata (4 errors, 1 warning)
+Name: Test-B README for Chromium metadata (3 errors, 1 warning)
 SHORT NAME: metadata-test-invalid
 URL: file://home/drive/chromium/src/metadata
 Version:0
@@ -34,10 +34,21 @@ Description:
 
 Local Modifications:     None.
 
+These are the expected errors (here for reference only):
+1. Description is empty.
+
+2. Required field 'License File' is missing.
+
+3. URL is invalid.
+
+warnings:
+
+1. Version is '0'.
+
 -------------------- DEPENDENCY DIVIDER --------------------
 -------------------- DEPENDENCY DIVIDER --------------------
 
-Name: Test-C README for Chromium metadata (5 errors, 1 warning)
+Name: Test-C README for Chromium metadata (4 errors, 1 warning)
 URL: https://www.example.com/first
 URL: https://www.example.com/second
 Version: N/A
@@ -47,4 +58,19 @@ Security Critical: yes
 
 Description:
 Test metadata with multiple entries for one field, and
-missing a mandatory field.
+missing a mandatory field.
+These are the expected errors (here for reference only):
+
+1. Required field 'License Android Compatible' is missing.
+
+2. Required field 'License File' is missing.
+
+3. Required field 'Shipped' is missing.
+
+4. Repeated fields: URL (2)
+
+warnings:
+1. License has a license not in the allowlist.
+(see https://source.chromium.org/chromium/chromiu
+m/tools/depot_tools/+/main:metadata/fields/custom/license_al
+lowlist.py). Licenses not allowlisted: 'Custom license'.

metadata/tests/data/README.chromium.test.reciprocal-license

@@ -0,0 +1,7 @@
+Name: Open Sauce Package
+URL: https://example.com
+Version: 1.0p.3N
+License: MPL-2.0
+License File: LICENSE
+Security Critical: no
+Shipped: yes

metadata/tests/dependency_metadata_test.py

@@ -1,3 +1,4 @@
+
 #!/usr/bin/env vpython3
 # Copyright (c) 2023 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
@@ -368,5 +369,32 @@ class DependencyValidationTest(unittest.TestCase):
         )
         self.assertEqual(len(results), 0)
 
+    def test_all_licenses_allowlisted(self):
+        """Test that a single allowlisted license returns True."""
+        dependency = dm.DependencyMetadata()
+        # "MPL-2.0" is a reciprocal license, i.e. only allowed in open source projects.
+        self.assertTrue(dependency.all_licenses_allowlisted("MIT", False))
+        self.assertTrue(dependency.all_licenses_allowlisted("MIT, Apache-2.0", False))
+        self.assertTrue(dependency.all_licenses_allowlisted("MPL-2.0", True))
+        self.assertFalse(dependency.all_licenses_allowlisted("InvalidLicense", False))
+        self.assertFalse(dependency.all_licenses_allowlisted("MIT, InvalidLicense", False))
+        self.assertFalse(dependency.all_licenses_allowlisted("", False))
+        self.assertFalse(dependency.all_licenses_allowlisted("MPL-2.0", False))
+
+
+    def test_only_open_source_licenses(self):
+        """Test that only open source licenses are returned."""
+        dependency = dm.DependencyMetadata()
+        self.assertEqual(dependency.only_open_source_licenses(""), [])
+        self.assertEqual(dependency.only_open_source_licenses("MIT"), [])
+        self.assertEqual(dependency.only_open_source_licenses("MPL-2.0"), ["MPL-2.0"])
+        result = dependency.only_open_source_licenses("MIT, MPL-2.0")
+        self.assertEqual(result, ["MPL-2.0"])
+        result = dependency.only_open_source_licenses("MPL-2.0, APSL-2.0")
+        self.assertEqual(set(result), {"MPL-2.0", "APSL-2.0"})
+        # Test with mix of invalid and valid licenses
+        result = dependency.only_open_source_licenses("InvalidLicense, MPL-2.0")
+        self.assertEqual(result, ["MPL-2.0"])
+
 if __name__ == "__main__":
     unittest.main()

metadata/tests/parse_test.py

@@ -77,7 +77,7 @@ class ParseTest(unittest.TestCase):
                  "     https://www.example.com/parser"),
                 ("Version", "1.0.12"),
                 ("Date", "2020-12-03"),
-                ('License', 'Apache-2.0 and MIT and not_an_spdx'),
+                ('License', 'Apache-2.0, MIT'),
                 ("License File", "LICENSE"),
                 ("Security Critical", "yes"),
                 ("Shipped", "yes"),
@@ -96,7 +96,7 @@ class ParseTest(unittest.TestCase):
             all_metadata[1].get_entries(),
             [
                 ("Name",
-                 "Test-B README for Chromium metadata (4 errors, 1 warning)"),
+                 "Test-B README for Chromium metadata (3 errors, 1 warning)"),
                 ("SHORT NAME", "metadata-test-invalid"),
                 ("URL", "file://home/drive/chromium/src/metadata"),
                 ("Version", "0"),
@@ -108,7 +108,7 @@ class ParseTest(unittest.TestCase):
                 ("Local Modifications", "None."),
             ],
         )
-        self.assertEqual((24, 35),
+        self.assertEqual((24, 46),
                          all_metadata[1].get_first_and_last_line_number())
 
         # Check repeated fields persist in the metadata's entries.
@@ -116,18 +116,34 @@ class ParseTest(unittest.TestCase):
             all_metadata[2].get_entries(),
             [
                 ("Name",
-                 "Test-C README for Chromium metadata (5 errors, 1 warning)"),
+                 "Test-C README for Chromium metadata (4 errors, 1 warning)"),
                 ("URL", "https://www.example.com/first"),
                 ("URL", "https://www.example.com/second"),
                 ("Version", "N/A"),
                 ("Date", "2020-12-03"),
                 ("License", "Custom license"),
                 ("Security Critical", "yes"),
-                ("Description", "Test metadata with multiple entries for one "
-                 "field, and\nmissing a mandatory field."),
+                ("Description", """Test metadata with multiple entries for one field, and
+missing a mandatory field.
+These are the expected errors (here for reference only):
+
+1. Required field 'License Android Compatible' is missing.
+
+2. Required field 'License File' is missing.
+
+3. Required field 'Shipped' is missing.
+
+4. Repeated fields: URL (2)
+
+warnings:
+1. License has a license not in the allowlist.
+(see https://source.chromium.org/chromium/chromiu
+m/tools/depot_tools/+/main:metadata/fields/custom/license_al
+lowlist.py). Licenses not allowlisted: 'Custom license'."""),
+
             ],
         )
-        self.assertEqual((40, 50),
+        self.assertEqual((51, 76),
                          all_metadata[2].get_first_and_last_line_number())
 
     def test_parse_multiple_local_modifications(self):

metadata/tests/validate_test.py

@@ -56,7 +56,7 @@ class ValidateContentTest(unittest.TestCase):
             source_file_dir=_SOURCE_FILE_DIR,
             repo_root_dir=_THIS_DIR,
         )
-        self.assertEqual(len(results), 11)
+        self.assertEqual(len(results), 9)
         error_count = 0
         warning_count = 0
         for result in results:
@@ -64,8 +64,8 @@ class ValidateContentTest(unittest.TestCase):
                 error_count += 1
             else:
                 warning_count += 1
-        self.assertEqual(error_count, 8)
-        self.assertEqual(warning_count, 3)
+        self.assertEqual(error_count, 7)
+        self.assertEqual(warning_count, 2)
 
 
 class ValidateFileTest(unittest.TestCase):
@@ -94,7 +94,7 @@ class ValidateFileTest(unittest.TestCase):
             filepath=_INVALID_METADATA_FILEPATH,
             repo_root_dir=_THIS_DIR,
         )
-        self.assertEqual(len(results), 11)
+        self.assertEqual(len(results), 9)
         error_count = 0
         warning_count = 0
         for result in results:
@@ -102,8 +102,8 @@ class ValidateFileTest(unittest.TestCase):
                 error_count += 1
             else:
                 warning_count += 1
-        self.assertEqual(error_count, 8)
-        self.assertEqual(warning_count, 3)
+        self.assertEqual(error_count, 7)
+        self.assertEqual(warning_count, 2)
 
 
 class CheckFileTest(unittest.TestCase):
@@ -141,7 +141,7 @@ class CheckFileTest(unittest.TestCase):
         # self.assertEqual(len(errors), 7)
         # self.assertEqual(len(warnings), 2)
         self.assertEqual(len(errors), 0)
-        self.assertEqual(len(warnings), 11)
+        self.assertEqual(len(warnings), 9)
 
 
 class ValidationResultTest(unittest.TestCase):
@@ -219,5 +219,41 @@ class ValidationWithLineNumbers(unittest.TestCase):
                 self.assertEqual(r.get_lines(), [13])
 
 
+class ValidateReciprocalLicenseTest(unittest.TestCase):
+    """Tests that validate_content handles allowing reciprocal licenses correctly."""
+    def test_reciprocal_licenses(self):
+        # Test content with a reciprocal license (MPL-2.0).
+        reciprocal_license_metadata_filepath = os.path.join(_THIS_DIR, "data",
+            "README.chromium.test.reciprocal-license")
+        # Without is_open_source_project, should get a warning.
+        results = metadata.validate.validate_content(
+            content=gclient_utils.FileRead(reciprocal_license_metadata_filepath),
+            source_file_dir=_SOURCE_FILE_DIR,
+            repo_root_dir=_THIS_DIR,
+            is_open_source_project=False
+        )
+
+        license_errors = []
+        for result in results:
+            if not result.is_fatal() and "License has a license not in the allowlist" in result.get_reason():
+                license_errors.append(result)
+
+        self.assertEqual(len(license_errors), 1, "Should create an error when a reciprocal license is used in a non-open source project")
+
+        # With is_open_source_project=True, should be no warnings.
+        results = metadata.validate.validate_content(
+            content=gclient_utils.FileRead(reciprocal_license_metadata_filepath),
+            source_file_dir=_SOURCE_FILE_DIR,
+            repo_root_dir=_THIS_DIR,
+            is_open_source_project=True
+        )
+
+        license_errors = []
+        for result in results:
+            if not result.is_fatal() and "License has a license not in the allowlist" in result.get_reason():
+                license_errors.append(result)
+
+        self.assertEqual(len(license_errors), 0, "Should not create an error when a reciprocal license is used in an open source project")
+
 if __name__ == "__main__":
     unittest.main()

metadata/validate.py

@@ -25,8 +25,10 @@ _TRANSITION_PRESCRIPT = (
     "validation is enforced.\nThird party metadata issue:")
 
 
-def validate_content(content: str, source_file_dir: str,
-                     repo_root_dir: str) -> List[vr.ValidationResult]:
+def validate_content(content: str,
+                     source_file_dir: str,
+                     repo_root_dir: str,
+                     is_open_source_project: bool = False) -> List[vr.ValidationResult]:
     """Validate the content as a metadata file.
 
     Args:
@@ -37,6 +39,7 @@ def validate_content(content: str, source_file_dir: str,
                          construct file paths to license files.
         repo_root_dir: the repository's root directory; this is needed
                        to construct file paths to license files.
+        is_open_source_project: whether the project is open source.
 
     Returns: the validation results for the given content, sorted based
              severity then message.
@@ -49,7 +52,10 @@ def validate_content(content: str, source_file_dir: str,
 
     for dependency in dependencies:
         dependency_results = dependency.validate(
-            source_file_dir=source_file_dir, repo_root_dir=repo_root_dir)
+            source_file_dir=source_file_dir,
+            repo_root_dir=repo_root_dir,
+            is_open_source_project=is_open_source_project,
+        )
         results.extend(dependency_results)
     return sorted(results)
 
@@ -68,6 +74,7 @@ def validate_file(
     filepath: str,
     repo_root_dir: str,
     reader: Callable[[str], Union[str, bytes]] = None,
+    is_open_source_project: bool = False,
 ) -> List[vr.ValidationResult]:
     """Validate the item located at the given filepath is a valid
     dependency metadata file.
@@ -79,6 +86,8 @@ def validate_file(
                        to construct file paths to license files.
         reader (optional): callable function/method to read the content
                            of the file.
+        is_open_source_project: whether to allow reciprocal licenses.
+                            This should only be True for open source projects.
 
     Returns: the validation results for the given filepath and its
              contents, if it exists.
@@ -104,15 +113,17 @@ def validate_file(
         source_file_dir = os.path.dirname(filepath)
         return validate_content(content=content,
                                 source_file_dir=source_file_dir,
-                                repo_root_dir=repo_root_dir)
+                                repo_root_dir=repo_root_dir,
+                                is_open_source_project=is_open_source_project)
 
 
 def check_file(
     filepath: str,
     repo_root_dir: str,
     reader: Callable[[str], Union[str, bytes]] = None,
+    is_open_source_project: bool = False,
 ) -> Tuple[List[str], List[str]]:
-    """Run metadata validation on the given filepath, and return all 
+    """Run metadata validation on the given filepath, and return all
     validation errors and validation warnings.
 
     Args:
@@ -122,6 +133,8 @@ def check_file(
                        to construct file paths to license files.
         reader (optional): callable function/method to read the content
                            of the file.
+        is_open_source_project: whether to allow reciprocal licenses.
+                            This should only be True for open source projects.
 
     Returns:
         error_messages: the fatal validation issues present in the file;
@@ -131,7 +144,8 @@ def check_file(
     """
     results = validate_file(filepath=filepath,
                             repo_root_dir=repo_root_dir,
-                            reader=reader)
+                            reader=reader,
+                            is_open_source_project=is_open_source_project)
 
     error_messages = []
     warning_messages = []

presubmit_canned_checks.py

@@ -894,6 +894,7 @@ def CheckChromiumDependencyMetadata(input_api, output_api, file_filter=None):
             filepath=f.AbsoluteLocalPath(),
             repo_root_dir=repo_root_dir,
             reader=input_api.ReadFile,
+            is_open_source_project=True,
         )
 
         for warning in warnings: