Require unambiguous, typed metadata in research export receipts

scripts/export_research_package.py

@@ -23,6 +23,14 @@
 FORMAT = 'research-package-export-v1'
 
 
+def unique_json_keys(pairs):
+    result = {}
+    for key, value in pairs:
+        require(key not in result, 'duplicate export JSON key: ' + key)
+        result[key] = value
+    return result
+
+
 def bundle_relative_path(name):
     canonical_relative_path(name)
     # A POSIX-relative name such as C:/file or C:file can discard the extraction
@@ -38,7 +46,8 @@ def inventory(root, package, ref, input_ref=None):
     # Read declarations from the publication, never the working manifest.
     with closing(git_blobs(root, [commit + ':' + package + '/VERIFY.json'])) as rows:
         values = list(rows)
-    manifest = json.loads(values[0][1])
+    manifest = json.loads(values[0][1], object_pairs_hook=unique_json_keys)
+    require(isinstance(manifest, dict), 'VERIFY.json must be an object')
     inputs = manifest.get('inputs')
     require(isinstance(inputs, list), 'VERIFY.json inputs must be a list')
     for name in inputs:
@@ -137,7 +146,7 @@ def check_bundle(path):
             with archive.extractfile(item) as data:
                 if item.name == RECEIPT:
                     require(item.size <= 16 * 1024 * 1024, 'oversized export receipt')
-                    receipt = json.load(data)
+                    receipt = json.load(data, object_pairs_hook=unique_json_keys)
                 else:
                     for block in iter(lambda: data.read(1024 * 1024), b''):
                         digest.update(block)
@@ -146,7 +155,7 @@ def check_bundle(path):
     require_file_tree(seen)
     require(isinstance(receipt, dict) and receipt.get('format') == FORMAT, 'missing or invalid export receipt')
     require(isinstance(receipt.get('commit'), str) and
-            re.fullmatch(r'[0-9a-f]{40,64}', receipt['commit']), 'invalid commit label')
+            re.fullmatch(r'(?:[0-9a-f]{40}|[0-9a-f]{64})', receipt['commit']), 'invalid commit label')
     if 'input_commit' in receipt:
         require(isinstance(receipt['input_commit'], str) and
                 re.fullmatch(r'(?:[0-9a-f]{40}|[0-9a-f]{64})', receipt['input_commit']), 'invalid input commit label')
@@ -157,6 +166,9 @@ def check_bundle(path):
     expected = {}
     for row in files:
         require(isinstance(row, dict), 'invalid receipt row')
+        require(type(row.get('size')) is int and row['size'] >= 0, 'invalid receipt size')
+        require(type(row.get('mode')) is int and row['mode'] in (0o644, 0o755),
+                'invalid receipt mode')
         name = row.get('path')
         bundle_relative_path(name)
         require(name != RECEIPT and name not in expected, 'duplicate or reserved receipt path')

tests/test_research_export_metadata.py

@@ -0,0 +1,72 @@
+"""Archive receipts must have unambiguous keys and exact integer fields."""
+import hashlib
+import json
+from pathlib import Path
+import sys
+import tarfile
+import tempfile
+import unittest
+from unittest.mock import patch
+
+sys.path.insert(0, str(Path(__file__).resolve().parents[1] / 'scripts'))
+import export_research_package as exporter
+sys.path.pop(0)
+
+
+class ReceiptMetadataTests(unittest.TestCase):
+    def receipt(self):
+        return {'format': exporter.FORMAT, 'commit': 'a' * 40, 'package': 'research/fixture',
+                'files': [{'path': 'research/fixture/VERIFY.json', 'mode': 0o644,
+                           'size': 0, 'sha256': hashlib.sha256(b'').hexdigest()}]}
+
+    def check(self, receipt, raw=None):
+        with tempfile.TemporaryDirectory() as tmp:
+            path = Path(tmp) / 'bundle.tar'
+            with tarfile.open(path, 'w') as archive:
+                archive.addfile(*exporter.member('research/fixture/VERIFY.json', b''))
+                archive.addfile(*exporter.member(exporter.RECEIPT,
+                                raw if raw is not None else json.dumps(receipt).encode()))
+            return exporter.check_bundle(path)
+
+    def test_invalid_committed_manifest_fails_before_tree_inventory(self):
+        for raw, message in ((b'[]', 'VERIFY.json must be an object'),
+                             (b'{"inputs":[],"inputs":[]}', 'duplicate export JSON key')):
+            rows = (row for row in [('b' * 40, raw)])
+            with patch.object(exporter, 'git', return_value=b'a' * 40) as git, \
+                 patch.object(exporter, 'git_blobs', return_value=rows):
+                with self.assertRaisesRegex(ValueError, message):
+                    exporter.inventory(Path('.'), 'research/fixture', 'HEAD')
+            self.assertEqual(git.call_count, 1)
+
+    def test_boolean_and_float_sizes_are_not_integer_receipts(self):
+        for bad in (False, 0.0, '0', -1, None):
+            with self.subTest(size=bad):
+                receipt = self.receipt(); receipt['files'][0]['size'] = bad
+                with self.assertRaisesRegex(ValueError, 'invalid receipt size'):
+                    self.check(receipt)
+
+    def test_float_mode_is_not_an_integer_mode(self):
+        receipt = self.receipt(); receipt['files'][0]['mode'] = float(0o644)
+        with self.assertRaisesRegex(ValueError, 'invalid receipt mode'):
+            self.check(receipt)
+
+    def test_only_actual_git_object_id_lengths_are_accepted(self):
+        for length in (39, 41, 48, 63, 65):
+            receipt = self.receipt(); receipt['commit'] = 'a' * length
+            with self.subTest(length=length), self.assertRaisesRegex(ValueError, 'invalid commit label'):
+                self.check(receipt)
+        for length in (40, 64):
+            receipt = self.receipt(); receipt['commit'] = 'a' * length
+            self.assertEqual(self.check(receipt), receipt)
+
+    def test_duplicate_top_level_and_nested_keys_are_rejected(self):
+        receipt = self.receipt(); raw = json.dumps(receipt)
+        variants = [raw[:-1] + ',"commit":"' + 'b' * 40 + '"}',
+                    raw.replace('"size": 0', '"size": 99, "size": 0')]
+        for text in variants:
+            with self.subTest(text=text), self.assertRaisesRegex(ValueError, 'duplicate export JSON key'):
+                self.check(receipt, text.encode())
+
+
+if __name__ == '__main__':
+    unittest.main()