From 7810a8ca4f8555bbdaeaca02b14ca8ceff84b230 Mon Sep 17 00:00:00 2001 From: Quratulain-bilal Date: Fri, 31 Jul 2026 17:07:25 +0500 Subject: [PATCH 1/3] fix: use incremental hashing in CatalogDescriptor.get_hash() to avoid loading entire file into memory CatalogDescriptor.get_hash() called fh.read() which loads the entire file into memory before hashing. Use incremental hashlib.sha256().update() with 64 KiB chunks to prevent unbounded memory allocation on large catalog descriptor files. --- src/specify_cli/integrations/catalog.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/specify_cli/integrations/catalog.py b/src/specify_cli/integrations/catalog.py index 1794caad83..49d2545d91 100644 --- a/src/specify_cli/integrations/catalog.py +++ b/src/specify_cli/integrations/catalog.py @@ -841,5 +841,8 @@ def tools(self) -> List[Dict[str, Any]]: def get_hash(self) -> str: """SHA-256 hash of the descriptor file.""" + h = hashlib.sha256() with open(self.path, "rb") as fh: - return f"sha256:{hashlib.sha256(fh.read()).hexdigest()}" + for chunk in iter(lambda: fh.read(65536), b""): + h.update(chunk) + return f"sha256:{h.hexdigest()}" From b57f0dc7a9c22a437c8622fad866d1ac3e9c2c32 Mon Sep 17 00:00:00 2001 From: Quratulain-bilal Date: Tue, 11 Aug 2026 02:08:34 +0500 Subject: [PATCH 2/3] fix: add regression test for incremental chunked hashing in CatalogDescriptor.get_hash() Add test_get_hash_incremental_chunking that: - Creates a 200 KiB+ YAML descriptor to force multiple 64 KiB chunks - Verifies the digest matches hashlib.sha256(content).hexdigest() - Rejects any unbounded f.read() call by patching builtins.open - Asserts at least 3 reads of <= 65536 bytes occurred --- .../integrations/test_integration_catalog.py | 51 +++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/tests/integrations/test_integration_catalog.py b/tests/integrations/test_integration_catalog.py index e8a9029db4..b198f07b43 100644 --- a/tests/integrations/test_integration_catalog.py +++ b/tests/integrations/test_integration_catalog.py @@ -646,6 +646,57 @@ def test_get_hash(self, tmp_path): h = desc.get_hash() assert h.startswith("sha256:") + def test_get_hash_incremental_chunking(self, tmp_path): + """Regression: get_hash() must read in bounded chunks, not f.read(). + + Creates a file larger than 64 KiB to exercise multiple chunk + iterations, verifies the digest matches hashlib.sha256(content), + and asserts that raw read() is never called with no size limit. + """ + import hashlib + from unittest.mock import patch + + # Build valid YAML content larger than 64 KiB. The padding field + # inflates the file without breaking YAML structure. + padding = "P" * (200 * 1024) # 200 KiB + data = {**VALID_DESCRIPTOR, "description": padding} + content = yaml.dump(data).encode("utf-8") + assert len(content) > 65536 # Ensure multi-chunk coverage. + + p = tmp_path / "integration.yml" + p.write_bytes(content) + desc = IntegrationDescriptor(p) + + # Spy on read() to reject unbounded calls. + original_open = open + read_sizes: list[int] = [] + + def tracking_open(path, *args, **kwargs): + fh = original_open(path, *args, **kwargs) + if (args and args[0] == "rb") or kwargs.get("mode") == "rb": + orig_read = fh.read + + def tracking_read(n=-1): + if n == -1 or n is None: + raise RuntimeError( + "f.read() called without size limit — " + "use bounded chunked reads instead" + ) + read_sizes.append(n) + return orig_read(n) + + fh.read = tracking_read + return fh + + with patch("builtins.open", side_effect=tracking_open): + result = desc.get_hash() + + expected = f"sha256:{hashlib.sha256(content).hexdigest()}" + assert result == expected + # Verify bounded reads happened (at least 3 chunks of <= 65536 bytes). + assert len(read_sizes) >= 3 + assert all(s <= 65536 for s in read_sizes) + def test_tools_accessor(self, tmp_path): data = {**VALID_DESCRIPTOR, "requires": { "speckit_version": ">=0.6.0", From b27b28fd3e9861b515d29a76c75865b810c1a830 Mon Sep 17 00:00:00 2001 From: Quratulain-bilal Date: Sun, 16 Aug 2026 02:30:00 +0500 Subject: [PATCH 3/3] fix: use read-only proxy for tracking open() in chunked hash test Assigning to fh.read on a _io.BufferedReader raises AttributeError because the attribute is read-only. Wrap the file in a proxy that intercepts read() while delegating everything else to the real file. --- .../integrations/test_integration_catalog.py | 43 +++++++++++++------ 1 file changed, 30 insertions(+), 13 deletions(-) diff --git a/tests/integrations/test_integration_catalog.py b/tests/integrations/test_integration_catalog.py index b198f07b43..f7a6fb2e01 100644 --- a/tests/integrations/test_integration_catalog.py +++ b/tests/integrations/test_integration_catalog.py @@ -667,25 +667,42 @@ def test_get_hash_incremental_chunking(self, tmp_path): p.write_bytes(content) desc = IntegrationDescriptor(p) - # Spy on read() to reject unbounded calls. + # Spy on read() to reject unbounded calls. We cannot assign + # directly to fh.read on a BufferedReader (read-only attribute), + # so wrap the file in a proxy that intercepts read() while + # delegating everything else — including context-manager cleanup + # — to the real file object. original_open = open read_sizes: list[int] = [] + class _ReadTrackingProxy: + """Proxy that intercepts read() calls on a file object.""" + + def __init__(self, fh): + self._fh = fh + + def read(self, n=-1): + if n == -1 or n is None: + raise RuntimeError( + "f.read() called without size limit — " + "use bounded chunked reads instead" + ) + read_sizes.append(n) + return self._fh.read(n) + + def __getattr__(self, name): + return getattr(self._fh, name) + + def __enter__(self): + return self + + def __exit__(self, *args): + return self._fh.__exit__(*args) + def tracking_open(path, *args, **kwargs): fh = original_open(path, *args, **kwargs) if (args and args[0] == "rb") or kwargs.get("mode") == "rb": - orig_read = fh.read - - def tracking_read(n=-1): - if n == -1 or n is None: - raise RuntimeError( - "f.read() called without size limit — " - "use bounded chunked reads instead" - ) - read_sizes.append(n) - return orig_read(n) - - fh.read = tracking_read + return _ReadTrackingProxy(fh) return fh with patch("builtins.open", side_effect=tracking_open):