Skip to content

Commit 783678c

Browse files
authored
fix(provenance): verify every matched SPDX, not just the first (#11)
verify() checked only sorted(glob)[0], so when the SPDX glob matched several documents a broken or tampered SPDX that sorted later passed CI unexamined -- a silent pass, the worst failure mode for a provenance verifier. Loop over all matched documents, accumulating problems so one run reports every broken file; object-store integrity still runs once. Adds regression tests: a later dangling SPDX now fails the run, and two valid documents both verify.
1 parent 749cc03 commit 783678c

2 files changed

Lines changed: 85 additions & 19 deletions

File tree

provenance/bomsh_verify.py

Lines changed: 41 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -167,26 +167,45 @@ def verify(spdx_glob, omnibor_dir):
167167
spdx_paths = sorted(_glob.glob(spdx_glob))
168168
if not spdx_paths:
169169
return False, [f'no SPDX matched {spdx_glob!r}']
170-
spdx_path = spdx_paths[0]
171-
try:
172-
spdx_gitoids = load_spdx_gitoids(spdx_path)
173-
except (json.JSONDecodeError, ValueError) as e:
174-
return False, [f'could not load SPDX gitoids: {e}']
175-
if not spdx_gitoids:
176-
return False, [f'no gitoid externalRefs in {spdx_path}']
177170

178171
objects_dir = os.path.join(omnibor_dir, 'objects')
179-
180-
missing = check_resolvability(spdx_gitoids, objects_dir)
181-
if missing:
182-
for pkg_name, gid, obj in missing:
172+
ok = True
173+
total_gitoids = 0
174+
175+
# (A) EVERY matched SPDX must load and have all its gitoids resolve --
176+
# not just spdx_paths[0]. When the glob matches several documents (e.g.
177+
# one per product/version), verifying only the first lets a broken or
178+
# tampered SPDX that sorts later pass unexamined; for a provenance
179+
# verifier a silent pass is the failure to avoid. Problems are
180+
# accumulated (rather than returned on the first) so one run reports every
181+
# broken document.
182+
for spdx_path in spdx_paths:
183+
try:
184+
spdx_gitoids = load_spdx_gitoids(spdx_path)
185+
except (json.JSONDecodeError, ValueError) as e:
186+
messages.append(
187+
f'could not load SPDX gitoids from {spdx_path}: {e}')
188+
ok = False
189+
continue
190+
if not spdx_gitoids:
191+
messages.append(f'no gitoid externalRefs in {spdx_path}')
192+
ok = False
193+
continue
194+
missing = check_resolvability(spdx_gitoids, objects_dir)
195+
if missing:
196+
for pkg_name, gid, obj in missing:
197+
messages.append(
198+
f'DANGLING: {spdx_path}: {pkg_name} gitoid {gid} -> {obj}')
183199
messages.append(
184-
f'DANGLING: {pkg_name} gitoid {gid} -> {obj}')
200+
f'{len(missing)} SPDX gitoid(s) from {spdx_path} not present '
201+
f'in {objects_dir}/ (provenance bundle is broken)')
202+
ok = False
203+
continue
204+
total_gitoids += len(spdx_gitoids)
185205
messages.append(
186-
f'{len(missing)} SPDX gitoid(s) not present in '
187-
f'{objects_dir}/ (provenance bundle is broken)')
188-
return False, messages
206+
f'OK: {spdx_path}: {len(spdx_gitoids)} gitoid(s) verified')
189207

208+
# (B) Object-store integrity, once for the whole store.
190209
obj_count, bad = check_object_store_integrity(objects_dir)
191210
if bad:
192211
for obj, expected, actual in bad[:5]:
@@ -195,11 +214,14 @@ def verify(spdx_glob, omnibor_dir):
195214
messages.append(
196215
f'{len(bad)} object(s) in {objects_dir}/ failed gitoid '
197216
f'round-trip (object store is corrupt)')
198-
return False, messages
217+
ok = False
199218

200-
messages.append(f'OK: {len(spdx_gitoids)} gitoid(s) verified')
201-
messages.append(f' objects round-trip: {obj_count} blobs')
202-
return True, messages
219+
if ok:
220+
messages.append(
221+
f'OK: {len(spdx_paths)} SPDX file(s), {total_gitoids} '
222+
f'gitoid(s) verified')
223+
messages.append(f' objects round-trip: {obj_count} blobs')
224+
return ok, messages
203225

204226

205227
def main():

tests/test_gen_sbom.py

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2824,6 +2824,50 @@ def test_no_gitoid_externalrefs_fails(self):
28242824
any('no gitoid externalRefs' in m for m in messages),
28252825
messages)
28262826

2827+
def test_all_matched_spdx_are_verified_not_just_first(self):
2828+
# Regression: when the glob matches several SPDX documents, the
2829+
# verifier must check every one. A second document that sorts *after*
2830+
# the good one and carries a dangling gitoid must fail the run --
2831+
# verifying only spdx_paths[0] would let it pass silently.
2832+
with tempfile.TemporaryDirectory() as tmpdir:
2833+
fx = _BomshFixture(tmpdir) # omnibor.wolfssl-5.9.1 (valid)
2834+
bogus_gid = 'a' * 40 # well-formed hex, never staged
2835+
second = fx.tmpdir / 'omnibor.wolfssl-5.9.2.spdx.json'
2836+
second.write_text(json.dumps({'packages': [{
2837+
'name': 'wolfssl-later',
2838+
'externalRefs': [{
2839+
'referenceCategory': 'PERSISTENT-ID',
2840+
'referenceType': 'gitoid',
2841+
'referenceLocator': f'gitoid:blob:sha1:{bogus_gid}',
2842+
}],
2843+
}]}))
2844+
ok, messages = fx.verify()
2845+
self.assertFalse(ok, f'later dangling SPDX not caught: {messages}')
2846+
joined = '\n'.join(messages)
2847+
self.assertIn('DANGLING', joined)
2848+
self.assertIn(bogus_gid, joined)
2849+
self.assertIn(str(second), joined) # names the offending document
2850+
2851+
def test_multiple_valid_spdx_all_pass(self):
2852+
# Positive companion: two valid SPDX documents both verify, and the
2853+
# summary counts gitoids from both (not just the first).
2854+
with tempfile.TemporaryDirectory() as tmpdir:
2855+
fx = _BomshFixture(tmpdir) # 3 gitoids
2856+
second = fx.tmpdir / 'omnibor.wolfssl-5.9.2.spdx.json'
2857+
second.write_text(json.dumps({'packages': [{
2858+
'name': 'wolfssl',
2859+
'externalRefs': [{
2860+
'referenceCategory': 'PERSISTENT-ID',
2861+
'referenceType': 'gitoid',
2862+
'referenceLocator':
2863+
f'gitoid:blob:sha1:{fx.gitoids["wolfssl"]}',
2864+
}],
2865+
}]}))
2866+
ok, messages = fx.verify()
2867+
self.assertTrue(ok, messages)
2868+
self.assertIn('2 SPDX file(s), 4 gitoid(s) verified',
2869+
'\n'.join(messages))
2870+
28272871
def test_object_store_integrity_skips_non_blob_files(self):
28282872
# OmniBOR objects/ may contain housekeeping files at the root
28292873
# (info/, pack/, etc.) that are NOT blobs and must not be

0 commit comments

Comments
 (0)