Skip to content

fix: enforce fail-fast validation and security bounds in record_handoff() - #191

Merged
rajfirke merged 1 commit into
rajfirke:mainfrom
Reh1t:fix/Buffered-trail-log-break
Sep 21, 2026
Merged

rajfirke merged 1 commit into
rajfirke:mainfrom
Reh1t:fix/Buffered-trail-log-break

Conversation

@Reh1t

@Reh1t Reh1t commented Sep 20, 2026

Copy link
Copy Markdown
Contributor

Summary

This PR resolves issue #50 by explicitly rejecting provisional IDs (id=-1) in TrailAggregator.record_handoff().

While implementing the fail-fast check, I also hardened the aggregator against several critical graph/pipeline vulnerabilities (NaN injection, cyclic loops, memory exhaustion).

What was changed

  • Provisional ID Validation: Added strict id < 1 bounds checking with an explicit ValueError teaching developers to call .flush() in buffered mode.
  • Strict Type Enforcement: Blocked Floats, NaNs, and Booleans using type() is int, preventing database ProgrammingError crashes during gap detection.
  • Cycle Blocker: Explicitly blocked self-referential graph loops (A -> A) to protect downstream graph traversals from infinite recursion.
  • Circuit Breaker: Implemented a 100,000 edge memory limit to prevent runaway agents from silently exhausting server RAM.
  • Regression Testing: Appended test_record_handoff_rejects_provisional_ids and test_record_handoff_red_team_defenses to mathematically prove all defenses hold.

🚀 Future Architecture Observation

While implementing this fix, I noticed that the root cause of the -1 provisional ID friction in buffered mode is that HandoffEdge tightly couples to database record_id integers.
If we eventually migrated HandoffEdge (and annotate) to link via the cryptographic chain_hash instead of auto-incrementing IDs, developers could log records in buffered mode and instantly record handoffs in RAM using the hash, without ever needing to force a synchronous .flush(). This would make the multi-agent aggregator completely decoupled from database write-latencies!

Checklist

  • Tests added/updated for the change
  • ruff check src/ tests/ passes
  • ruff format --check src/ tests/ passes
  • mypy src/provena/ passes
  • pytest passes with no failures
  • CHANGELOG.md updated (if user-facing change)

Closes #50

…ff()

- Reject provisional IDs (`id=-1`) in `TrailAggregator.record_handoff()` with a clear ValueError.
- Enforce strict `int` type checking to block NaN/Float injections that crash the database.
- Block self-referential cycles (`A -> A`) from causing infinite recursion graphs.
- Introduce a 100,000 edge circuit breaker to prevent runaway agent memory exhaustion (DoS).
- Add robust `test_record_handoff_red_team_defenses` regression tests.

Fixes rajfirke#50
@rajfirke

Copy link
Copy Markdown
Owner

Good fix for #50. Fail-fast on provisional IDs matches what we do in annotate(), and the tests cover buffered mode clearly. Thanks @Reh1t for the extra handoff guards and for the chain_hash idea on the issue thread.

@rajfirke
rajfirke merged commit 0f812ad into rajfirke:main Sep 21, 2026
6 checks passed
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.

Buffered trail.log() returns TrailRecord with id=-1 — breaks annotate() and handoffs

2 participants