Skip to content

Commit d33e44b

Browse files
committed
feat(doctor): report object integrity failures
1 parent 24a5915 commit d33e44b

2 files changed

Lines changed: 200 additions & 25 deletions

File tree

‎src/rgit/doctor.py‎

Lines changed: 83 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
from __future__ import annotations
22

3+
import hashlib
34
import json
45
from pathlib import Path
56
from typing import Any
@@ -145,17 +146,20 @@ def _check_feature_payloads(store, findings: list[dict[str, Any]]) -> None:
145146
fid,
146147
)
147148
continue
148-
try:
149-
payload = _load_json_object(store, digest)
150-
except FileNotFoundError:
151-
_add(
152-
findings,
153-
"error",
154-
"missing_feature_payload_object",
155-
"feature payload_hash does not resolve to an object",
156-
fid,
157-
)
149+
payload_ok, payload_bytes = _check_object_reference(
150+
store,
151+
findings,
152+
digest,
153+
fid,
154+
kind="feature_payload",
155+
reference="feature payload_hash",
156+
load_bytes=True,
157+
)
158+
if not payload_ok:
158159
continue
160+
assert payload_bytes is not None
161+
try:
162+
payload = json.loads(payload_bytes)
159163
except (UnicodeDecodeError, json.JSONDecodeError):
160164
_add(
161165
findings,
@@ -203,14 +207,14 @@ def _check_run_artifacts(store, findings: list[dict[str, Any]]) -> None:
203207
rid,
204208
)
205209
continue
206-
if not store.objects.path_for(digest).exists():
207-
_add(
208-
findings,
209-
"error",
210-
"missing_run_artifact_object",
211-
"run artifact_hash does not resolve to an object",
212-
rid,
213-
)
210+
_check_object_reference(
211+
store,
212+
findings,
213+
digest,
214+
rid,
215+
kind="run_artifact",
216+
reference="run artifact_hash",
217+
)
214218

215219

216220
def _check_proposals(store, findings: list[dict[str, Any]]) -> None:
@@ -225,13 +229,14 @@ def _check_proposals(store, findings: list[dict[str, Any]]) -> None:
225229
"proposal has no diff_ref",
226230
pid,
227231
)
228-
elif not store.objects.path_for(diff_ref).exists():
229-
_add(
232+
else:
233+
_check_object_reference(
234+
store,
230235
findings,
231-
"error",
232-
"missing_proposal_diff_object",
233-
"proposal diff_ref does not resolve to an object",
236+
diff_ref,
234237
pid,
238+
kind="proposal_diff",
239+
reference="proposal diff_ref",
235240
)
236241
try:
237242
candidates = json.loads(row["candidates"])
@@ -343,8 +348,61 @@ def _expected_endpoint(edge_type: str, side: str) -> str:
343348
return "known"
344349

345350

346-
def _load_json_object(store, digest: str) -> Any:
347-
return json.loads(store.objects.get(digest))
351+
def _check_object_reference(
352+
store,
353+
findings: list[dict[str, Any]],
354+
digest: str,
355+
subject: str,
356+
*,
357+
kind: str,
358+
reference: str,
359+
load_bytes: bool = False,
360+
) -> tuple[bool, bytes | None]:
361+
"""Return validity and, when requested, the exact bytes that were hashed."""
362+
if not store.objects.is_valid_digest(digest):
363+
_add(
364+
findings,
365+
"error",
366+
f"invalid_{kind}_reference",
367+
f"{reference} is not a canonical lowercase sha256 digest",
368+
subject,
369+
)
370+
return False, None
371+
try:
372+
if load_bytes:
373+
data = store.objects.get(digest)
374+
matches = hashlib.sha256(data).hexdigest() == digest
375+
else:
376+
data = None
377+
matches = store.objects.verify(digest)
378+
except FileNotFoundError:
379+
_add(
380+
findings,
381+
"error",
382+
f"missing_{kind}_object",
383+
f"{reference} does not resolve to an object",
384+
subject,
385+
)
386+
return False, None
387+
except OSError:
388+
_add(
389+
findings,
390+
"error",
391+
f"unreadable_{kind}_object",
392+
f"{kind.replace('_', ' ')} object cannot be read",
393+
subject,
394+
)
395+
return False, None
396+
if not matches:
397+
_add(
398+
findings,
399+
"error",
400+
f"corrupt_{kind}_object",
401+
f"{kind.replace('_', ' ')} object does not match sha256 {digest}",
402+
subject,
403+
)
404+
return False, None
405+
return True, data
348406

349407

350408
def _add(

‎tests/test_doctor.py‎

Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,65 @@ def test_doctor_reports_missing_feature_payload_object(git_repo):
103103
assert "missing_feature_payload_object" in _codes(report, level="error")
104104

105105

106+
def test_doctor_reports_corrupt_feature_payload_object(git_repo):
107+
from rgit.doctor import run_doctor
108+
109+
store = Store.init(git_repo)
110+
fid = store.add_feature(_cap())
111+
payload_hash = store.conn.execute(
112+
"SELECT payload_hash FROM features WHERE id=?", (fid,)
113+
).fetchone()["payload_hash"]
114+
_object_path(store, payload_hash).write_bytes(b"[]")
115+
116+
report = run_doctor(store)
117+
118+
assert report["ok"] is False
119+
assert "corrupt_feature_payload_object" in _codes(report, level="error")
120+
assert "malformed_feature_payload_json" not in _codes(report)
121+
122+
123+
def test_doctor_hashes_the_same_feature_payload_bytes_it_parses(
124+
git_repo, monkeypatch
125+
):
126+
from rgit.doctor import run_doctor
127+
128+
store = Store.init(git_repo)
129+
fid = store.add_feature(_cap())
130+
payload_hash = store.conn.execute(
131+
"SELECT payload_hash FROM features WHERE id=?", (fid,)
132+
).fetchone()["payload_hash"]
133+
real_get = store.objects.get
134+
135+
def tamper_before_read(digest):
136+
_object_path(store, payload_hash).write_bytes(b"[]")
137+
return real_get(digest)
138+
139+
monkeypatch.setattr(store.objects, "get", tamper_before_read)
140+
141+
report = run_doctor(store)
142+
143+
assert report["ok"] is False
144+
assert "corrupt_feature_payload_object" in _codes(report, level="error")
145+
assert "malformed_feature_payload_json" not in _codes(report)
146+
147+
148+
def test_doctor_reports_feature_payload_read_race(git_repo, monkeypatch):
149+
from rgit.doctor import run_doctor
150+
151+
store = Store.init(git_repo)
152+
store.add_feature(_cap())
153+
154+
def deny_read(digest):
155+
raise PermissionError("simulated read race")
156+
157+
monkeypatch.setattr(store.objects, "get", deny_read)
158+
159+
report = run_doctor(store)
160+
161+
assert report["ok"] is False
162+
assert "unreadable_feature_payload_object" in _codes(report, level="error")
163+
164+
106165
def test_doctor_reports_missing_run_artifact_object(git_repo):
107166
from rgit.doctor import run_doctor
108167

@@ -116,6 +175,20 @@ def test_doctor_reports_missing_run_artifact_object(git_repo):
116175
assert "missing_run_artifact_object" in _codes(report, level="error")
117176

118177

178+
def test_doctor_reports_corrupt_run_artifact_object(git_repo):
179+
from rgit.doctor import run_doctor
180+
181+
store = Store.init(git_repo)
182+
artifact_hash = store.objects.put(b"artifact")
183+
_run(store, artifact=artifact_hash)
184+
_object_path(store, artifact_hash).write_bytes(b"tampered")
185+
186+
report = run_doctor(store)
187+
188+
assert report["ok"] is False
189+
assert "corrupt_run_artifact_object" in _codes(report, level="error")
190+
191+
119192
def test_doctor_reports_missing_proposal_diff_object(git_repo):
120193
from rgit.doctor import run_doctor
121194

@@ -129,6 +202,50 @@ def test_doctor_reports_missing_proposal_diff_object(git_repo):
129202
assert "missing_proposal_diff_object" in _codes(report, level="error")
130203

131204

205+
def test_doctor_reports_corrupt_proposal_diff_object(git_repo):
206+
from rgit.doctor import run_doctor
207+
208+
store = Store.init(git_repo)
209+
diff_ref = store.objects.put(b"diff")
210+
_proposal(store, diff=diff_ref)
211+
_object_path(store, diff_ref).write_bytes(b"tampered")
212+
213+
report = run_doctor(store)
214+
215+
assert report["ok"] is False
216+
assert "corrupt_proposal_diff_object" in _codes(report, level="error")
217+
218+
219+
def test_doctor_reports_invalid_object_reference_without_path_lookup(git_repo):
220+
from rgit.doctor import run_doctor
221+
222+
store = Store.init(git_repo)
223+
_run(store, artifact="../outside")
224+
225+
report = run_doctor(store)
226+
227+
assert report["ok"] is False
228+
assert "invalid_run_artifact_reference" in _codes(report, level="error")
229+
230+
231+
def test_doctor_reports_unreadable_object(git_repo, monkeypatch):
232+
from rgit.doctor import run_doctor
233+
234+
store = Store.init(git_repo)
235+
artifact_hash = store.objects.put(b"artifact")
236+
_run(store, artifact=artifact_hash)
237+
238+
def deny_read(digest):
239+
raise PermissionError("simulated unreadable object")
240+
241+
monkeypatch.setattr(store.objects, "verify", deny_read)
242+
243+
report = run_doctor(store)
244+
245+
assert report["ok"] is False
246+
assert "unreadable_run_artifact_object" in _codes(report, level="error")
247+
248+
132249
def test_doctor_reports_malformed_proposal_candidates_json(git_repo):
133250
from rgit.doctor import run_doctor
134251

0 commit comments

Comments
 (0)