Skip to content

Commit 91bf2e6

Browse files
committed
fix(cli): defensive programming, JSON output, ASCII symbols, extended tests
1 parent 244a9bb commit 91bf2e6

14 files changed

Lines changed: 1795 additions & 435 deletions

File tree

packages/devqubit-engine/src/devqubit_engine/bundle/reader.py

Lines changed: 242 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,46 @@
2323

2424
logger = logging.getLogger(__name__)
2525

26+
# Maximum allowed uncompressed size for metadata files (10 MB)
27+
_MAX_METADATA_SIZE = 10 * 1024 * 1024
28+
29+
# Maximum allowed uncompressed size for objects (1 GB)
30+
_MAX_OBJECT_SIZE = 1024 * 1024 * 1024
31+
32+
33+
def _validate_digest(digest: str) -> str | None:
34+
"""
35+
Validate digest format and return hex part.
36+
37+
Parameters
38+
----------
39+
digest : str
40+
Digest to validate.
41+
42+
Returns
43+
-------
44+
str or None
45+
Lowercase hex part if valid, None otherwise.
46+
"""
47+
if not isinstance(digest, str) or not digest.startswith("sha256:"):
48+
return None
49+
50+
hex_part = digest[7:].strip().lower()
51+
if len(hex_part) != 64:
52+
return None
53+
54+
try:
55+
int(hex_part, 16)
56+
except ValueError:
57+
return None
58+
59+
return hex_part
60+
61+
62+
def _digest_to_path(hex_part: str) -> str:
63+
"""Convert hex digest to bundle object path."""
64+
return f"objects/sha256/{hex_part[:2]}/{hex_part}"
65+
2666

2767
def is_bundle_path(path: Any) -> bool:
2868
"""
@@ -69,6 +109,9 @@ class BundleStore:
69109
Objects are stored under ``objects/sha256/<prefix>/<hex>`` and addressed
70110
by ``sha256:<hex>`` digests.
71111
112+
This class provides a partial implementation of ObjectStoreProtocol,
113+
supporting read-only operations only.
114+
72115
Parameters
73116
----------
74117
zf : zipfile.ZipFile
@@ -91,6 +134,10 @@ def __init__(self, zf: zipfile.ZipFile) -> None:
91134
self._zf = zf
92135
# Cache namelist for O(1) exists() checks
93136
self._names = frozenset(zf.namelist())
137+
# Cache ZipInfo for size lookups
138+
self._info_cache: dict[str, zipfile.ZipInfo] = {}
139+
for info in zf.infolist():
140+
self._info_cache[info.filename] = info
94141

95142
def get_bytes(self, digest: str) -> bytes:
96143
"""
@@ -113,25 +160,43 @@ def get_bytes(self, digest: str) -> bytes:
113160
ObjectNotFoundError
114161
If the object is not present in the bundle.
115162
"""
116-
if not isinstance(digest, str) or not digest.startswith("sha256:"):
163+
hex_part = _validate_digest(digest)
164+
if hex_part is None:
117165
raise ValueError(f"Invalid digest format: {digest!r}")
118166

119-
hex_part = digest[7:].strip().lower()
120-
if len(hex_part) != 64:
121-
raise ValueError(f"Invalid digest length: {digest!r}")
167+
path = _digest_to_path(hex_part)
122168

123-
try:
124-
int(hex_part, 16)
125-
except ValueError as e:
126-
raise ValueError(f"Invalid digest hex: {digest!r}") from e
127-
128-
path = f"objects/sha256/{hex_part[:2]}/{hex_part}"
169+
# Check for zip bomb before reading
170+
info = self._info_cache.get(path)
171+
if info is not None and info.file_size > _MAX_OBJECT_SIZE:
172+
raise ValueError(
173+
f"Object too large: {info.file_size} bytes " f"(max {_MAX_OBJECT_SIZE})"
174+
)
129175

130176
try:
131177
return self._zf.read(path)
132178
except KeyError as e:
133179
raise ObjectNotFoundError(f"sha256:{hex_part}") from e
134180

181+
def get_bytes_or_none(self, digest: str) -> bytes | None:
182+
"""
183+
Retrieve bytes by digest, returning None if not found.
184+
185+
Parameters
186+
----------
187+
digest : str
188+
Object identifier in the form ``sha256:<64 hex chars>``.
189+
190+
Returns
191+
-------
192+
bytes or None
193+
Raw object bytes, or None if not found or invalid digest.
194+
"""
195+
try:
196+
return self.get_bytes(digest)
197+
except (ObjectNotFoundError, ValueError):
198+
return None
199+
135200
def exists(self, digest: str) -> bool:
136201
"""
137202
Check if an object exists in the bundle.
@@ -147,40 +212,86 @@ def exists(self, digest: str) -> bool:
147212
True if the object exists in the bundle.
148213
Invalid digests return False.
149214
"""
150-
if not isinstance(digest, str) or not digest.startswith("sha256:"):
215+
hex_part = _validate_digest(digest)
216+
if hex_part is None:
151217
return False
152218

153-
hex_part = digest[7:].strip().lower()
154-
if len(hex_part) != 64:
155-
return False
219+
path = _digest_to_path(hex_part)
220+
return path in self._names
156221

157-
try:
158-
int(hex_part, 16)
159-
except ValueError:
160-
return False
222+
def get_size(self, digest: str) -> int:
223+
"""
224+
Get the uncompressed size of an object.
161225
162-
path = f"objects/sha256/{hex_part[:2]}/{hex_part}"
163-
return path in self._names
226+
Parameters
227+
----------
228+
digest : str
229+
Object identifier in the form ``sha256:<64 hex chars>``.
230+
231+
Returns
232+
-------
233+
int
234+
Uncompressed size in bytes.
235+
236+
Raises
237+
------
238+
ValueError
239+
If digest format is invalid.
240+
ObjectNotFoundError
241+
If the object is not present in the bundle.
242+
"""
243+
hex_part = _validate_digest(digest)
244+
if hex_part is None:
245+
raise ValueError(f"Invalid digest format: {digest!r}")
246+
247+
path = _digest_to_path(hex_part)
248+
info = self._info_cache.get(path)
164249

165-
def list_objects(self) -> Iterator[str]:
250+
if info is None:
251+
raise ObjectNotFoundError(f"sha256:{hex_part}")
252+
253+
return info.file_size
254+
255+
def list_digests(self, prefix: str | None = None) -> Iterator[str]:
166256
"""
167257
Iterate over all stored object digests in the bundle.
168258
259+
Parameters
260+
----------
261+
prefix : str, optional
262+
Filter by digest prefix (e.g., "sha256:ab").
263+
169264
Yields
170265
------
171266
str
172267
Digests in the form ``sha256:<64 hex chars>``.
173268
"""
174269
for name in self._names:
175-
if name.startswith("objects/sha256/") and len(name.split("/")) == 4:
176-
hex_part = name.split("/")[-1].strip().lower()
177-
if len(hex_part) != 64:
178-
continue
179-
try:
180-
int(hex_part, 16)
181-
except ValueError:
182-
continue
183-
yield f"sha256:{hex_part}"
270+
if not name.startswith("objects/sha256/"):
271+
continue
272+
273+
parts = name.split("/")
274+
if len(parts) != 4:
275+
continue
276+
277+
hex_part = parts[-1].strip().lower()
278+
if len(hex_part) != 64:
279+
continue
280+
281+
try:
282+
int(hex_part, 16)
283+
except ValueError:
284+
continue
285+
286+
digest = f"sha256:{hex_part}"
287+
288+
if prefix is not None and not digest.startswith(prefix):
289+
continue
290+
291+
yield digest
292+
293+
# Alias for compatibility
294+
list_objects = list_digests
184295

185296

186297
class Bundle:
@@ -268,6 +379,48 @@ def __repr__(self) -> str:
268379
status = "open" if self._zf else "closed"
269380
return f"Bundle({self.path!r}, {status})"
270381

382+
def _read_json(self, filename: str, max_size: int = _MAX_METADATA_SIZE) -> Any:
383+
"""
384+
Read and parse a JSON file from the bundle.
385+
386+
Parameters
387+
----------
388+
filename : str
389+
Name of file in bundle.
390+
max_size : int
391+
Maximum allowed uncompressed size.
392+
393+
Returns
394+
-------
395+
Any
396+
Parsed JSON content.
397+
398+
Raises
399+
------
400+
RuntimeError
401+
If bundle is not open.
402+
ValueError
403+
If file is too large or not valid JSON.
404+
"""
405+
if self._zf is None:
406+
raise RuntimeError("Bundle not open")
407+
408+
# Check size before reading
409+
try:
410+
info = self._zf.getinfo(filename)
411+
if info.file_size > max_size:
412+
raise ValueError(
413+
f"{filename} too large: {info.file_size} bytes (max {max_size})"
414+
)
415+
except KeyError as e:
416+
raise ValueError(f"Missing required file: {filename}") from e
417+
418+
data = self._zf.read(filename)
419+
try:
420+
return json.loads(data.decode("utf-8"))
421+
except (json.JSONDecodeError, UnicodeDecodeError) as e:
422+
raise ValueError(f"Invalid JSON in {filename}: {e}") from e
423+
271424
@property
272425
def manifest(self) -> dict[str, Any]:
273426
"""
@@ -285,9 +438,7 @@ def manifest(self) -> dict[str, Any]:
285438
If the bundle is not open.
286439
"""
287440
if self._manifest is None:
288-
if self._zf is None:
289-
raise RuntimeError("Bundle not open")
290-
self._manifest = json.loads(self._zf.read("manifest.json").decode("utf-8"))
441+
self._manifest = self._read_json("manifest.json")
291442
return self._manifest
292443

293444
@property
@@ -306,9 +457,7 @@ def run_record(self) -> dict[str, Any]:
306457
If the bundle is not open.
307458
"""
308459
if self._run_record is None:
309-
if self._zf is None:
310-
raise RuntimeError("Bundle not open")
311-
self._run_record = json.loads(self._zf.read("run.json").decode("utf-8"))
460+
self._run_record = self._read_json("run.json")
312461
return self._run_record
313462

314463
@property
@@ -353,7 +502,7 @@ def list_objects(self) -> list[str]:
353502
list of str
354503
Digests in the form ``sha256:<64 hex chars>``.
355504
"""
356-
return list(self.store.list_objects())
505+
return list(self.store.list_digests())
357506

358507
def get_artifact_kinds(self) -> list[str]:
359508
"""
@@ -394,3 +543,59 @@ def get_adapter(self) -> str:
394543
Adapter name, or empty string if not present.
395544
"""
396545
return self.run_record.get("adapter", "")
546+
547+
def get_status(self) -> str:
548+
"""
549+
Get run status from run record.
550+
551+
Returns
552+
-------
553+
str
554+
Run status (RUNNING, FINISHED, FAILED, KILLED),
555+
or "UNKNOWN" if not present.
556+
"""
557+
info = self.run_record.get("info", {})
558+
if isinstance(info, dict):
559+
return info.get("status", "UNKNOWN")
560+
return "UNKNOWN"
561+
562+
def get_created_at(self) -> str | None:
563+
"""
564+
Get run creation timestamp.
565+
566+
Returns
567+
-------
568+
str or None
569+
ISO 8601 timestamp, or None if not present.
570+
"""
571+
return self.run_record.get("created_at")
572+
573+
def validate_objects(self) -> tuple[list[str], list[str]]:
574+
"""
575+
Validate that all artifact digests exist in bundle.
576+
577+
Returns
578+
-------
579+
tuple of (list, list)
580+
(present_digests, missing_digests)
581+
"""
582+
present: list[str] = []
583+
missing: list[str] = []
584+
585+
arts = self.run_record.get("artifacts", []) or []
586+
if not isinstance(arts, list):
587+
return present, missing
588+
589+
for art in arts:
590+
if not isinstance(art, dict):
591+
continue
592+
digest = art.get("digest", "")
593+
if not digest:
594+
continue
595+
596+
if self.store.exists(digest):
597+
present.append(digest)
598+
else:
599+
missing.append(digest)
600+
601+
return present, missing

0 commit comments

Comments
 (0)