diff --git a/scripts/export_research_package.py b/scripts/export_research_package.py index 8166dba88e..3c4b5ca6ef 100644 --- a/scripts/export_research_package.py +++ b/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') diff --git a/tests/test_research_export_metadata.py b/tests/test_research_export_metadata.py new file mode 100644 index 0000000000..3ed09bc891 --- /dev/null +++ b/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()