Skip to content

fix: drop evidence entries whose evidence_data serializes away#17

Merged
olivrg merged 1 commit into
mainfrom
fix/evidence-data-serialize-drop
Jun 18, 2026
Merged

fix: drop evidence entries whose evidence_data serializes away#17
olivrg merged 1 commit into
mainfrom
fix/evidence-data-serialize-drop

Conversation

@olivrg

@olivrg olivrg commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

Resolves the external review's single blocking finding.

The bug

safeJsonSnapshot uses JSON.parse(JSON.stringify(entry)). When an extracted evidence_data value is a function or symbol, JSON.stringify silently omits that property from the object (it does not throw). The snapshot therefore returns ok: true with evidence_data gone — a structurally malformed entry { evidence_key } missing its required field, which was then sent on /audit.

This was reachable: extractByPath returns found: true for a function/symbol value (it only rejects undefined). The malformed entry would 400 the audit; because audit failures are best-effort and swallowed, a legitimately-executed call would degrade into a missing finalization — surfacing server-side as evaluation_expired, a false tamper/bypass signal.

The fix

After snapshotting, also drop any entry whose evidence_data did not survive the round-trip (=== undefined). Precise, no false drops:

  • extractByPath already excludes undefined inputs, and BigInt/circular refs throw (already dropped via the existing catch), so the function/symbol drop was the only path that slipped through.
  • A legitimate null value survives the round-trip and is kept.

Verification

pnpm verify green: 198 tests (was 196), 0 type errors, build ✓. New test asserts a function-valued evidence path drops only its own entry while the serializable sibling survives — complementing the existing BigInt (throws) per-entry test.

External-review blocking finding. safeJsonSnapshot used
JSON.parse(JSON.stringify(entry)); when an extracted evidence_data value is a
function or symbol, JSON.stringify silently OMITS that property (it does not
throw), so the snapshot succeeded with the required evidence_data gone — a
malformed { evidence_key }-only entry that was then sent on /audit.

extractByPath already rejects undefined, and BigInt/circular refs throw (already
dropped), so the function/symbol drop was the one path that slipped through. The
malformed entry would 400 the audit; since audit failures are best-effort and
swallowed, a legitimately-executed call would degrade into a missing
finalization (server-side evaluation_expired — a false tamper signal).

Fix: after snapshotting, also drop any entry whose evidence_data did not survive
the round-trip (=== undefined). A legitimate null value survives.
@olivrg olivrg merged commit 63a959b into main Jun 18, 2026
1 check passed
@olivrg olivrg deleted the fix/evidence-data-serialize-drop branch June 18, 2026 14:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant