⚡ Bolt: O(1) Resolution Evidence Verification & Blockchain Chaining#625
⚡ Bolt: O(1) Resolution Evidence Verification & Blockchain Chaining#625RohanExploit wants to merge 1 commit into
Conversation
- Implemented HMAC-SHA256 hash chaining for ResolutionEvidence records. - Optimized verify_evidence to use O(1) SQLAlchemy queries (.count() and .first()) instead of loading all records. - Added resolution_last_hash_cache for O(1) record creation. - Added O(1) blockchain-verify endpoint for resolution evidence. - Synchronized ResolutionProofToken model with test suite expectations (nonce, valid_from, valid_until). - Updated init_db.py with necessary migrations for new columns and indexes.
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
❌ Deploy Preview for fixmybharat failed. Why did it fail? →
|
🙏 Thank you for your contribution, @RohanExploit!PR Details:
Quality Checklist:
Review Process:
Note: The maintainers will monitor code quality and ensure the overall project flow isn't broken. |
📝 WalkthroughWalkthroughThis PR implements blockchain integrity hashing for resolution proof evidence by adding integrity chain columns to models, migration logic for schema updates, a cache layer for performance, and a new API endpoint to verify evidence using blockchain-style integrity chains. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Router
participant Service
participant Cache
participant Database
Client->>Router: POST /resolution-proof/submit
Router->>Service: submit_evidence(...)
Service->>Cache: get resolution_last_hash_cache
alt Cache hit
Service->>Service: use cached prev_hash
else Cache miss
Service->>Database: query ResolutionEvidence ORDER BY id DESC LIMIT 1
Database-->>Service: latest integrity_hash or None
Service->>Service: use retrieved prev_hash
end
Service->>Service: compute integrity_hash = HMAC-SHA256(evidence_hash|gps_coords|prev_hash)
Service->>Database: create ResolutionEvidence with integrity_hash & previous_integrity_hash
Database-->>Service: commit successful
Service->>Cache: update resolution_last_hash_cache = new integrity_hash
Service-->>Router: success response
Router-->>Client: HTTP 200
sequenceDiagram
participant Client
participant Router
participant Service
participant Database
Client->>Router: GET /{evidence_id}/blockchain-verify
Router->>Database: fetch ResolutionEvidence by id
Database-->>Router: evidence record
Router->>Service: _sign_payload(evidence_hash|gps_coords|previous_integrity_hash)
Service-->>Router: computed_hash
Router->>Router: compare computed_hash vs integrity_hash
alt integrity_hash is None
Router->>Router: is_valid = False (unsealed/legacy)
else integrity_hash exists
Router->>Router: is_valid = (computed_hash == integrity_hash)
end
Router-->>Client: BlockchainVerificationResponse
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR extends the backend “Resolution Proof” feature with blockchain-style integrity chaining for ResolutionEvidence and adds an O(1)-memory optimization for evidence verification, alongside a new API endpoint to verify an individual evidence record’s integrity.
Changes:
- Add
integrity_hash+previous_integrity_hashtoResolutionEvidence, compute and store chained HMAC on evidence submission, and expose a blockchain verification endpoint. - Optimize
verify_evidence(grievance_id)to avoid loading all evidence rows into memory by usingcount()+ fetching only the latest record. - Add migrations for new DB columns (resolution evidence integrity fields; resolution proof token validity fields) and introduce a new
resolution_last_hash_cache.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| backend/routers/resolution_proof.py | Adds /{evidence_id}/blockchain-verify endpoint for evidence integrity verification. |
| backend/resolution_proof_service.py | Computes chained HMAC integrity hashes during evidence submission; optimizes verification query pattern. |
| backend/models.py | Adds integrity fields to ResolutionEvidence; adds missing validity/nonce fields to ResolutionProofToken. |
| backend/init_db.py | Adds migration steps for new resolution evidence and token columns + index. |
| backend/cache.py | Adds resolution_last_hash_cache singleton. |
| .jules/bolt.md | Documents a learned contract between models and tests for Resolution Proof tokens. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Performance Boost: Use thread-safe cache to eliminate DB query for last hash | ||
| prev_hash = resolution_last_hash_cache.get("last_hash") | ||
| if prev_hash is None: | ||
| # Cache miss: Fetch only the last hash from DB | ||
| prev_record = db.query(ResolutionEvidence.integrity_hash).order_by(ResolutionEvidence.id.desc()).first() | ||
| prev_hash = prev_record[0] if prev_record and prev_record[0] else "" | ||
| resolution_last_hash_cache.set(data=prev_hash, key="last_hash") | ||
|
|
There was a problem hiding this comment.
resolution_last_hash_cache stores a single global last_hash without validating it against the current DB tail. In multi-worker deployments or under concurrent submissions, two requests can read the same cached prev_hash and commit different rows, creating a forked/incorrect chain (and stale cache can chain to an old hash). Consider mirroring GrievanceService’s pattern: cache both last_id+last_hash and re-check the DB’s latest (id, integrity_hash) before using the cache; ideally compute prev_hash inside the same transaction as the insert to avoid races.
| # Performance Boost: Use thread-safe cache to eliminate DB query for last hash | |
| prev_hash = resolution_last_hash_cache.get("last_hash") | |
| if prev_hash is None: | |
| # Cache miss: Fetch only the last hash from DB | |
| prev_record = db.query(ResolutionEvidence.integrity_hash).order_by(ResolutionEvidence.id.desc()).first() | |
| prev_hash = prev_record[0] if prev_record and prev_record[0] else "" | |
| resolution_last_hash_cache.set(data=prev_hash, key="last_hash") | |
| # Performance Boost with safety: cache DB tail (id + integrity_hash) and | |
| # validate it against the current DB state to avoid chaining to stale hashes. | |
| cache_entry = resolution_last_hash_cache.get("last_tail") | |
| cached_id: Optional[int] = None | |
| cached_hash: Optional[str] = None | |
| # Backward compatibility: older cache may store only a string hash under this key. | |
| if isinstance(cache_entry, dict): | |
| cached_id = cache_entry.get("id") | |
| cached_hash = cache_entry.get("hash") | |
| elif isinstance(cache_entry, str): | |
| cached_hash = cache_entry or "" | |
| # Always read the current DB tail to validate the cache. | |
| db_tail: Optional[Tuple[int, Optional[str]]] = db.query( | |
| ResolutionEvidence.id, | |
| ResolutionEvidence.integrity_hash | |
| ).order_by(ResolutionEvidence.id.desc()).first() | |
| if db_tail: | |
| db_last_id, db_last_hash = db_tail | |
| db_last_hash = db_last_hash or "" | |
| else: | |
| db_last_id, db_last_hash = None, "" | |
| if cached_id is not None and cached_id == db_last_id and cached_hash == db_last_hash: | |
| # Cache is in sync with DB tail; safe to use. | |
| prev_hash = cached_hash or "" | |
| else: | |
| # Cache miss, mismatch, or legacy format; use DB tail and refresh cache. | |
| prev_hash = db_last_hash | |
| resolution_last_hash_cache.set( | |
| data={"id": db_last_id, "hash": prev_hash}, | |
| key="last_tail", | |
| ) |
| # Chaining logic: HMAC-SHA256(evidence_hash|gps_lat|gps_lon|prev_hash) | ||
| hash_content = f"{evidence_hash}|{gps_latitude}|{gps_longitude}|{prev_hash}" | ||
| integrity_hash = ResolutionProofService._sign_payload(hash_content) |
There was a problem hiding this comment.
The integrity hash payload is built via f-string interpolation of gps_latitude/gps_longitude floats. Float string representations can differ between the value at submission time and the value round-tripped through the DB, causing false integrity failures. Use a canonical encoding for numeric fields (e.g., fixed precision formatting or Decimal quantization) in both submit_evidence and blockchain verification.
| @@ -380,6 +394,8 @@ def submit_evidence( | |||
| metadata_bundle=metadata_bundle, | |||
| server_signature=server_signature, | |||
| verification_status=VerificationStatus.VERIFIED, | |||
| integrity_hash=integrity_hash, | |||
| previous_integrity_hash=prev_hash | |||
There was a problem hiding this comment.
The chaining payload only includes evidence_hash|gps_lat|gps_lon|prev_hash, so changes to other persisted fields (e.g., capture_timestamp, device_fingerprint_hash, server_signature, metadata_bundle) will not be detected by this blockchain seal. If the intent is “record integrity”, consider hashing a canonical form of the full immutable evidence bundle (or at least include the server_signature/metadata fields) to prevent undetected tampering of those columns.
| else: | ||
| is_valid = (computed_hash == evidence.integrity_hash) | ||
| message = ( | ||
| "Integrity verified. This evidence record is cryptographically sealed." | ||
| if is_valid | ||
| else "Integrity check failed! The evidence data does not match its cryptographic seal." | ||
| ) |
There was a problem hiding this comment.
computed_hash == evidence.integrity_hash is used for comparison. Since this is an HMAC-based value, prefer hmac.compare_digest (as already done in _verify_signature) to avoid timing side-channels and keep comparisons consistent across the service.
| valid_from = Column(DateTime, default=lambda: datetime.datetime.now(datetime.timezone.utc)) | ||
| valid_until = Column(DateTime, nullable=True) |
There was a problem hiding this comment.
valid_from/valid_until are required by schemas and used unconditionally in token validation/signature payloads, but the model allows valid_until to be NULL. After migration, existing rows will have NULL in these new columns unless backfilled, which can trigger runtime errors when accessing .tzinfo or calling .isoformat(). Consider making these non-nullable (or adding a fallback/backfill strategy using generated_at/expires_at) to keep model invariants aligned with service expectations.
| valid_from = Column(DateTime, default=lambda: datetime.datetime.now(datetime.timezone.utc)) | |
| valid_until = Column(DateTime, nullable=True) | |
| valid_from = Column(DateTime, nullable=False, default=lambda: datetime.datetime.now(datetime.timezone.utc)) | |
| valid_until = Column(DateTime, nullable=False, default=lambda: datetime.datetime.now(datetime.timezone.utc)) |
| if not column_exists("resolution_proof_tokens", "nonce"): | ||
| conn.execute(text("ALTER TABLE resolution_proof_tokens ADD COLUMN nonce VARCHAR")) | ||
| logger.info("Added nonce column to resolution_proof_tokens") | ||
|
|
There was a problem hiding this comment.
The migration adds valid_from, valid_until, and nonce columns but doesn’t backfill them for existing resolution_proof_tokens rows. If any pre-migration tokens are still in use (or accessed during deployment), these NULLs can break token validation/signature checks. Consider updating existing rows after adding columns (e.g., set valid_from = generated_at and valid_until = expires_at, and generate a nonce if required for signature verification).
| # Backfill new columns for existing resolution_proof_tokens rows | |
| # Use existing generated_at/expires_at for legacy tokens where available. | |
| if column_exists("resolution_proof_tokens", "valid_from") and column_exists("resolution_proof_tokens", "generated_at"): | |
| conn.execute(text(""" | |
| UPDATE resolution_proof_tokens | |
| SET valid_from = generated_at | |
| WHERE valid_from IS NULL AND generated_at IS NOT NULL | |
| """)) | |
| if column_exists("resolution_proof_tokens", "valid_until") and column_exists("resolution_proof_tokens", "expires_at"): | |
| conn.execute(text(""" | |
| UPDATE resolution_proof_tokens | |
| SET valid_until = expires_at | |
| WHERE valid_until IS NULL AND expires_at IS NOT NULL | |
| """)) | |
| if column_exists("resolution_proof_tokens", "nonce"): | |
| # Generate a nonce for legacy rows that do not have one. | |
| # This expression uses PostgreSQL built-ins to derive a pseudo-random string. | |
| conn.execute(text(""" | |
| UPDATE resolution_proof_tokens | |
| SET nonce = md5(random()::text || clock_timestamp()::text) | |
| WHERE nonce IS NULL | |
| """)) |
| @router.get("/{evidence_id}/blockchain-verify", response_model=BlockchainVerificationResponse) | ||
| def verify_evidence_blockchain( | ||
| evidence_id: int, | ||
| db: Session = Depends(get_db) | ||
| ): | ||
| """ | ||
| Verify the cryptographic integrity of an individual resolution evidence record. | ||
| Optimized: Uses previous_integrity_hash column for O(1) verification. | ||
| """ | ||
| try: | ||
| from backend.models import ResolutionEvidence | ||
| evidence = db.query( | ||
| ResolutionEvidence.evidence_hash, | ||
| ResolutionEvidence.gps_latitude, | ||
| ResolutionEvidence.gps_longitude, | ||
| ResolutionEvidence.integrity_hash, | ||
| ResolutionEvidence.previous_integrity_hash | ||
| ).filter(ResolutionEvidence.id == evidence_id).first() | ||
|
|
||
| if not evidence: | ||
| raise HTTPException(status_code=404, detail="Evidence not found") | ||
|
|
||
| # Determine previous hash (O(1) from stored column) | ||
| prev_hash = evidence.previous_integrity_hash or "" | ||
|
|
||
| # Recompute hash based on current data and previous hash | ||
| # Chaining logic: HMAC-SHA256(evidence_hash|gps_lat|gps_lon|prev_hash) | ||
| hash_content = f"{evidence.evidence_hash}|{evidence.gps_latitude}|{evidence.gps_longitude}|{prev_hash}" | ||
| computed_hash = ResolutionProofService._sign_payload(hash_content) |
There was a problem hiding this comment.
No tests appear to cover the new resolution-evidence blockchain seal and the /{evidence_id}/blockchain-verify endpoint (e.g., tests/test_resolution_proof.py has no references to integrity_hash or blockchain-verify). Adding a couple of tests (happy path + tamper case) would help prevent regressions in the chaining payload/canonicalization and cache/prev-hash logic.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/resolution_proof_service.py (1)
372-411:⚠️ Potential issue | 🟠 MajorRace condition in integrity hash chaining can cause chain forks.
Two concurrent
submit_evidencecalls can read the sameprev_hashfrom cache (line 374), compute their integrity hashes using the same previous hash, and both commit successfully. This creates two evidence records with identicalprevious_integrity_hashvalues—a fork in the chain that violates the blockchain invariant.The
ThreadSafeCachelock only protects individual get/set operations, not the entire read-compute-write sequence spanning lines 374-411.Consider one of these mitigations:
- Database-level locking: Use
SELECT ... FOR UPDATEon the last evidence record to serialize chain extensions- Optimistic locking: Add a unique constraint on
previous_integrity_hashand handleIntegrityErrorwith retry- Application-level lock: Use a dedicated lock around the entire chaining operation (coarse but simple)
The
grievance_service.py(lines 87-115) shows a partial mitigation by caching bothlast_hashandlast_idthen validating against the DB, but even that doesn't fully prevent the race.Suggested mitigation using unique constraint + retry
+from sqlalchemy.exc import IntegrityError + # In submit_evidence, wrap the commit in a retry loop: + max_retries = 3 + for attempt in range(max_retries): + try: db.commit() db.refresh(evidence) + break + except IntegrityError: + db.rollback() + if attempt == max_retries - 1: + raise ValueError("Concurrent evidence submission detected. Please retry.") + # Re-fetch the latest hash and recompute + prev_record = db.query(ResolutionEvidence.integrity_hash).order_by(ResolutionEvidence.id.desc()).first() + prev_hash = prev_record[0] if prev_record and prev_record[0] else "" + hash_content = f"{evidence_hash}|{gps_latitude}|{gps_longitude}|{prev_hash}" + integrity_hash = ResolutionProofService._sign_payload(hash_content) + evidence.integrity_hash = integrity_hash + evidence.previous_integrity_hash = prev_hashThis requires adding a unique constraint on
previous_integrity_hashin the model/migration.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/resolution_proof_service.py` around lines 372 - 411, The chaining is vulnerable to races because resolution_last_hash_cache.get/read + compute + db.commit isn't atomic; fix by adding an optimistic-locking retry: add a UNIQUE constraint on ResolutionEvidence.previous_integrity_hash in the model/migration, and wrap the create/commit logic in ResolutionProofService.submit_evidence (or the function containing the shown diff) with a retry loop that catches sqlalchemy.exc.IntegrityError on db.commit, rolls back, re-fetches the latest prev_hash (from DB or resolution_last_hash_cache), recomputes integrity_hash using ResolutionProofService._sign_payload, and retries up to N attempts before failing; also ensure after a successful commit you update resolution_last_hash_cache.set(data=integrity_hash, key="last_hash").
🧹 Nitpick comments (2)
backend/routers/resolution_proof.py (2)
236-244: Consider moving the import to module level.The
from backend.models import ResolutionEvidenceimport inside the function works but is unconventional. SinceResolutionEvidenceis already imported inresolution_proof_service.py(which this module imports), and there's no circular import risk here, moving it to the top of the file would be cleaner.Proposed refactor
At the top of the file (around line 17):
from backend.models import ResolutionEvidenceThen remove the inline import at line 237.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/routers/resolution_proof.py` around lines 236 - 244, Move the inline import of ResolutionEvidence out of the function and into the module imports at the top of the file: add "from backend.models import ResolutionEvidence" alongside the other top-level imports and remove the in-function "from backend.models import ResolutionEvidence" line so the query that references ResolutionEvidence (in the block using db.query(...).filter(ResolutionEvidence.id == evidence_id).first()) uses the module-level symbol.
275-279: Useraise ... from efor proper exception chaining.Per Ruff B904, re-raising exceptions within an
exceptblock should usefrom e(orfrom None) to preserve the exception chain and make debugging easier.Proposed fix
except HTTPException: raise except Exception as e: logger.error(f"Error verifying evidence blockchain for {evidence_id}: {e}", exc_info=True) - raise HTTPException(status_code=500, detail="Failed to verify evidence integrity") + raise HTTPException(status_code=500, detail="Failed to verify evidence integrity") from e🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/routers/resolution_proof.py` around lines 275 - 279, The except block in resolution_proof.py that catches Exception as e should re-raise the new HTTPException with proper exception chaining; update the final line to raise HTTPException(status_code=500, detail="Failed to verify evidence integrity") from e so the original exception (e) is preserved in the chain (keep the existing logger.error(...) call and leave the bare except HTTPException: raise as-is).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/init_db.py`:
- Around line 209-234: Main issue: new migrations that add integrity_hash and
previous_integrity_hash to ResolutionEvidence and valid_from/valid_until to
ResolutionProofToken are required by submit_evidence in
resolution_proof_service.py (submit_evidence writes
ResolutionEvidence.integrity_hash and .previous_integrity_hash and reads
ResolutionProofToken.valid_from and .valid_until); fix by re-enabling the
database migration at startup—uncomment the migrate_db() call in main.py (or
call migrate_db() before any code that may call submit_evidence) so the ALTER
TABLE/CREATE INDEX steps in init_db.py run before deployment, or alternatively
ensure migrations are executed manually prior to running the service.
- Around line 224-230: The migration adds DATETIME columns to
resolution_proof_tokens which is SQLite-specific; update the ALTER TABLE
statements in init_db.py to use TIMESTAMP instead of DATETIME so they work on
PostgreSQL as well — locate the block using
column_exists("resolution_proof_tokens", "valid_from") and
column_exists("resolution_proof_tokens", "valid_until") and change the
conn.execute(text("ALTER TABLE resolution_proof_tokens ADD COLUMN valid_from
DATETIME")) and the corresponding valid_until statement to use TIMESTAMP; keep
the logger.info calls and behavior unchanged.
---
Outside diff comments:
In `@backend/resolution_proof_service.py`:
- Around line 372-411: The chaining is vulnerable to races because
resolution_last_hash_cache.get/read + compute + db.commit isn't atomic; fix by
adding an optimistic-locking retry: add a UNIQUE constraint on
ResolutionEvidence.previous_integrity_hash in the model/migration, and wrap the
create/commit logic in ResolutionProofService.submit_evidence (or the function
containing the shown diff) with a retry loop that catches
sqlalchemy.exc.IntegrityError on db.commit, rolls back, re-fetches the latest
prev_hash (from DB or resolution_last_hash_cache), recomputes integrity_hash
using ResolutionProofService._sign_payload, and retries up to N attempts before
failing; also ensure after a successful commit you update
resolution_last_hash_cache.set(data=integrity_hash, key="last_hash").
---
Nitpick comments:
In `@backend/routers/resolution_proof.py`:
- Around line 236-244: Move the inline import of ResolutionEvidence out of the
function and into the module imports at the top of the file: add "from
backend.models import ResolutionEvidence" alongside the other top-level imports
and remove the in-function "from backend.models import ResolutionEvidence" line
so the query that references ResolutionEvidence (in the block using
db.query(...).filter(ResolutionEvidence.id == evidence_id).first()) uses the
module-level symbol.
- Around line 275-279: The except block in resolution_proof.py that catches
Exception as e should re-raise the new HTTPException with proper exception
chaining; update the final line to raise HTTPException(status_code=500,
detail="Failed to verify evidence integrity") from e so the original exception
(e) is preserved in the chain (keep the existing logger.error(...) call and
leave the bare except HTTPException: raise as-is).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c61004e2-5b22-4a89-892c-eff3f2c3650f
📒 Files selected for processing (6)
.jules/bolt.mdbackend/cache.pybackend/init_db.pybackend/models.pybackend/resolution_proof_service.pybackend/routers/resolution_proof.py
| # Resolution Evidence Table Migrations | ||
| if inspector.has_table("resolution_evidence"): | ||
| if not column_exists("resolution_evidence", "integrity_hash"): | ||
| conn.execute(text("ALTER TABLE resolution_evidence ADD COLUMN integrity_hash VARCHAR")) | ||
| logger.info("Added integrity_hash column to resolution_evidence") | ||
|
|
||
| if not column_exists("resolution_evidence", "previous_integrity_hash"): | ||
| conn.execute(text("ALTER TABLE resolution_evidence ADD COLUMN previous_integrity_hash VARCHAR")) | ||
| logger.info("Added previous_integrity_hash column to resolution_evidence") | ||
|
|
||
| if not index_exists("resolution_evidence", "ix_resolution_evidence_previous_integrity_hash"): | ||
| conn.execute(text("CREATE INDEX IF NOT EXISTS ix_resolution_evidence_previous_integrity_hash ON resolution_evidence (previous_integrity_hash)")) | ||
|
|
||
| # Resolution Proof Tokens Table Migrations | ||
| if inspector.has_table("resolution_proof_tokens"): | ||
| if not column_exists("resolution_proof_tokens", "valid_from"): | ||
| conn.execute(text("ALTER TABLE resolution_proof_tokens ADD COLUMN valid_from DATETIME")) | ||
| logger.info("Added valid_from column to resolution_proof_tokens") | ||
|
|
||
| if not column_exists("resolution_proof_tokens", "valid_until"): | ||
| conn.execute(text("ALTER TABLE resolution_proof_tokens ADD COLUMN valid_until DATETIME")) | ||
| logger.info("Added valid_until column to resolution_proof_tokens") | ||
|
|
||
| if not column_exists("resolution_proof_tokens", "nonce"): | ||
| conn.execute(text("ALTER TABLE resolution_proof_tokens ADD COLUMN nonce VARCHAR")) | ||
| logger.info("Added nonce column to resolution_proof_tokens") |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the current state of migrate_db() in main.py
rg -n 'migrate_db' backend/main.py -C3Repository: RohanExploit/VishwaGuru
Length of output: 953
🏁 Script executed:
rg -n 'def submit_evidence' backend/ -A 20Repository: RohanExploit/VishwaGuru
Length of output: 1540
🏁 Script executed:
rg -n 'integrity_hash|previous_integrity_hash|valid_from|valid_until' backend/ --type pyRepository: RohanExploit/VishwaGuru
Length of output: 10464
🏁 Script executed:
rg -n 'def submit_evidence' backend/resolution_proof_service.py -A 100Repository: RohanExploit/VishwaGuru
Length of output: 4559
🏁 Script executed:
rg -n 'class ResolutionEvidence' backend/models.py -A 20Repository: RohanExploit/VishwaGuru
Length of output: 1232
🏁 Script executed:
rg -n 'class ResolutionEvidence' backend/models.py -A 35Repository: RohanExploit/VishwaGuru
Length of output: 2014
🏁 Script executed:
sed -n '385,410p' backend/resolution_proof_service.pyRepository: RohanExploit/VishwaGuru
Length of output: 984
🏁 Script executed:
grep -n "resolution_proof_tokens" backend/models.py | head -20Repository: RohanExploit/VishwaGuru
Length of output: 208
🏁 Script executed:
sed -n '299,320p' backend/models.pyRepository: RohanExploit/VishwaGuru
Length of output: 1228
Re-enable migrations in main.py before deployment — submit_evidence requires new database columns.
The migrate_db() call is commented out at line 89 in main.py. The submit_evidence function in resolution_proof_service.py writes to integrity_hash and previous_integrity_hash on ResolutionEvidence (lines 397–398) and reads valid_from and valid_until from ResolutionProofToken (lines 331–332). If existing deployments have these tables without the new columns, database errors will occur when attempting to insert or read evidence records.
Either uncomment migrate_db() at line 89 or ensure the migration is run manually before deployment.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/init_db.py` around lines 209 - 234, Main issue: new migrations that
add integrity_hash and previous_integrity_hash to ResolutionEvidence and
valid_from/valid_until to ResolutionProofToken are required by submit_evidence
in resolution_proof_service.py (submit_evidence writes
ResolutionEvidence.integrity_hash and .previous_integrity_hash and reads
ResolutionProofToken.valid_from and .valid_until); fix by re-enabling the
database migration at startup—uncomment the migrate_db() call in main.py (or
call migrate_db() before any code that may call submit_evidence) so the ALTER
TABLE/CREATE INDEX steps in init_db.py run before deployment, or alternatively
ensure migrations are executed manually prior to running the service.
| if not column_exists("resolution_proof_tokens", "valid_from"): | ||
| conn.execute(text("ALTER TABLE resolution_proof_tokens ADD COLUMN valid_from DATETIME")) | ||
| logger.info("Added valid_from column to resolution_proof_tokens") | ||
|
|
||
| if not column_exists("resolution_proof_tokens", "valid_until"): | ||
| conn.execute(text("ALTER TABLE resolution_proof_tokens ADD COLUMN valid_until DATETIME")) | ||
| logger.info("Added valid_until column to resolution_proof_tokens") |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check what database type is configured in this project
rg -n 'DATABASE_URL|postgresql|sqlite' --type=py -C2Repository: RohanExploit/VishwaGuru
Length of output: 4228
🏁 Script executed:
# Check existing migrations in init_db.py to see what types are used
head -60 backend/init_db.py | cat -n
# Also check around line 52 mentioned in the scratchpad for comparison
sed -n '45,60p' backend/init_db.py | cat -n
# Check the full list of columns being added to understand patterns
rg -n "ALTER TABLE|ADD COLUMN" backend/init_db.py -A1 | head -40Repository: RohanExploit/VishwaGuru
Length of output: 5897
🏁 Script executed:
# Check main.py around line 89 to verify if migrations are disabled
sed -n '85,95p' backend/main.py | cat -nRepository: RohanExploit/VishwaGuru
Length of output: 802
🏁 Script executed:
# Get the actual lines 224-230 from init_db.py
sed -n '224,230p' backend/init_db.py | cat -nRepository: RohanExploit/VishwaGuru
Length of output: 667
🏁 Script executed:
# Check if there are any existing datetime columns or timestamp handling in models
rg -n "DATETIME|TIMESTAMP|datetime|timestamp" backend/models.py -B2 -A2 | head -50Repository: RohanExploit/VishwaGuru
Length of output: 2466
🏁 Script executed:
# Check database.py for any datetime handling
rg -n "DATETIME|TIMESTAMP|datetime|timestamp" backend/database.pyRepository: RohanExploit/VishwaGuru
Length of output: 49
🏁 Script executed:
# Verify SQLite compatibility with TIMESTAMP
python3 << 'EOF'
# Check if TIMESTAMP is valid in both SQLite and PostgreSQL
# This is general database knowledge, but let's verify behavior
# For SQLite: TIMESTAMP is accepted as a type affinity
# For PostgreSQL: TIMESTAMP is the standard datetime type
# Let's check if there's any type conversion logic in the codebase
import subprocess
result = subprocess.run(['rg', '-n', 'DateTime|TIMESTAMP|DATETIME', 'backend/', '-t', 'py'],
capture_output=True, text=True)
print("DateTime/TIMESTAMP references:")
print(result.stdout[:2000] if result.stdout else "No matches")
EOFRepository: RohanExploit/VishwaGuru
Length of output: 2098
🏁 Script executed:
# Check the full init_db.py to understand all datetime column migrations
rg -n "valid_from|valid_until" backend/init_db.py -B2 -A2Repository: RohanExploit/VishwaGuru
Length of output: 855
🏁 Script executed:
# Find all ALTER TABLE ADD COLUMN statements with datetime types in init_db.py
rg -n "ADD COLUMN.*DATETIME|ADD COLUMN.*TIMESTAMP" backend/init_db.pyRepository: RohanExploit/VishwaGuru
Length of output: 293
Change DATETIME to TIMESTAMP for PostgreSQL compatibility.
The code uses DATETIME which is SQLite-specific. PostgreSQL requires TIMESTAMP. Since the project supports both PostgreSQL (via DATABASE_URL) and SQLite, these columns should use TIMESTAMP to work across both databases. All other migrations in this file follow this cross-DB pattern using portable types (INTEGER, FLOAT, VARCHAR, TEXT).
Fix for PostgreSQL compatibility
if inspector.has_table("resolution_proof_tokens"):
if not column_exists("resolution_proof_tokens", "valid_from"):
- conn.execute(text("ALTER TABLE resolution_proof_tokens ADD COLUMN valid_from DATETIME"))
+ conn.execute(text("ALTER TABLE resolution_proof_tokens ADD COLUMN valid_from TIMESTAMP"))
logger.info("Added valid_from column to resolution_proof_tokens")
if not column_exists("resolution_proof_tokens", "valid_until"):
- conn.execute(text("ALTER TABLE resolution_proof_tokens ADD COLUMN valid_until DATETIME"))
+ conn.execute(text("ALTER TABLE resolution_proof_tokens ADD COLUMN valid_until TIMESTAMP"))
logger.info("Added valid_until column to resolution_proof_tokens")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if not column_exists("resolution_proof_tokens", "valid_from"): | |
| conn.execute(text("ALTER TABLE resolution_proof_tokens ADD COLUMN valid_from DATETIME")) | |
| logger.info("Added valid_from column to resolution_proof_tokens") | |
| if not column_exists("resolution_proof_tokens", "valid_until"): | |
| conn.execute(text("ALTER TABLE resolution_proof_tokens ADD COLUMN valid_until DATETIME")) | |
| logger.info("Added valid_until column to resolution_proof_tokens") | |
| if not column_exists("resolution_proof_tokens", "valid_from"): | |
| conn.execute(text("ALTER TABLE resolution_proof_tokens ADD COLUMN valid_from TIMESTAMP")) | |
| logger.info("Added valid_from column to resolution_proof_tokens") | |
| if not column_exists("resolution_proof_tokens", "valid_until"): | |
| conn.execute(text("ALTER TABLE resolution_proof_tokens ADD COLUMN valid_until TIMESTAMP")) | |
| logger.info("Added valid_until column to resolution_proof_tokens") |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/init_db.py` around lines 224 - 230, The migration adds DATETIME
columns to resolution_proof_tokens which is SQLite-specific; update the ALTER
TABLE statements in init_db.py to use TIMESTAMP instead of DATETIME so they work
on PostgreSQL as well — locate the block using
column_exists("resolution_proof_tokens", "valid_from") and
column_exists("resolution_proof_tokens", "valid_until") and change the
conn.execute(text("ALTER TABLE resolution_proof_tokens ADD COLUMN valid_from
DATETIME")) and the corresponding valid_until statement to use TIMESTAMP; keep
the logger.info calls and behavior unchanged.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
6 issues found across 6 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="backend/models.py">
<violation number="1" location="backend/models.py:308">
P1: `valid_until` is nullable in the model, but if it's used unconditionally in token validation or signature payloads (e.g., calling `.isoformat()` or checking `.tzinfo`), existing rows with `NULL` after migration will trigger `AttributeError` at runtime. Either make it non-nullable with a sensible default, or add a backfill strategy during migration.</violation>
</file>
<file name="backend/routers/resolution_proof.py">
<violation number="1" location="backend/routers/resolution_proof.py:262">
P2: Use `hmac.compare_digest()` instead of `==` for HMAC comparison to prevent timing side-channel attacks. The codebase already uses `hmac.compare_digest` in `_verify_signature` — this comparison should be consistent.</violation>
</file>
<file name="backend/resolution_proof_service.py">
<violation number="1" location="backend/resolution_proof_service.py:374">
P1: The cache stores a single global `last_hash` without any concurrency guard or DB-tail validation. In multi-worker or concurrent-request scenarios, two requests can read the same cached `prev_hash`, both commit with the same previous hash, and fork the chain. Additionally, the cache is only updated after commit, leaving a window where concurrent writers use stale data.
Consider caching both the last record ID and hash, and re-validating against the DB tail within the same transaction before using the cached value.</violation>
<violation number="2" location="backend/resolution_proof_service.py:382">
P1: Float-to-string interpolation via f-string (`{gps_latitude}|{gps_longitude}`) can produce different representations at submission time vs. after a database round-trip (e.g., `34.0` vs `34.00000000000001`). This will cause false integrity-check failures in the blockchain-verify endpoint. Use fixed-precision formatting (e.g., `f"{gps_latitude:.8f}"`) or `Decimal` quantization for canonical encoding.</violation>
</file>
<file name="backend/init_db.py">
<violation number="1" location="backend/init_db.py:225">
P2: Use `TIMESTAMP` instead of `DATETIME` for cross-database compatibility. The project supports both PostgreSQL (via `DATABASE_URL`) and SQLite. `DATETIME` is a SQLite-specific type; PostgreSQL requires `TIMESTAMP`. Other migrations in this file use portable types.</violation>
<violation number="2" location="backend/init_db.py:229">
P2: Same issue: use `TIMESTAMP` instead of `DATETIME` for PostgreSQL compatibility.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| authority_email = Column(String, nullable=True) | ||
| generated_at = Column(DateTime, default=lambda: datetime.datetime.now(datetime.timezone.utc)) | ||
| valid_from = Column(DateTime, default=lambda: datetime.datetime.now(datetime.timezone.utc)) | ||
| valid_until = Column(DateTime, nullable=True) |
There was a problem hiding this comment.
P1: valid_until is nullable in the model, but if it's used unconditionally in token validation or signature payloads (e.g., calling .isoformat() or checking .tzinfo), existing rows with NULL after migration will trigger AttributeError at runtime. Either make it non-nullable with a sensible default, or add a backfill strategy during migration.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At backend/models.py, line 308:
<comment>`valid_until` is nullable in the model, but if it's used unconditionally in token validation or signature payloads (e.g., calling `.isoformat()` or checking `.tzinfo`), existing rows with `NULL` after migration will trigger `AttributeError` at runtime. Either make it non-nullable with a sensible default, or add a backfill strategy during migration.</comment>
<file context>
@@ -300,7 +304,10 @@ class ResolutionProofToken(Base):
authority_email = Column(String, nullable=True)
generated_at = Column(DateTime, default=lambda: datetime.datetime.now(datetime.timezone.utc))
+ valid_from = Column(DateTime, default=lambda: datetime.datetime.now(datetime.timezone.utc))
+ valid_until = Column(DateTime, nullable=True)
expires_at = Column(DateTime, nullable=False)
+ nonce = Column(String, nullable=True)
</file context>
|
|
||
| # 5a. Blockchain feature: calculate integrity hash for the evidence record | ||
| # Performance Boost: Use thread-safe cache to eliminate DB query for last hash | ||
| prev_hash = resolution_last_hash_cache.get("last_hash") |
There was a problem hiding this comment.
P1: The cache stores a single global last_hash without any concurrency guard or DB-tail validation. In multi-worker or concurrent-request scenarios, two requests can read the same cached prev_hash, both commit with the same previous hash, and fork the chain. Additionally, the cache is only updated after commit, leaving a window where concurrent writers use stale data.
Consider caching both the last record ID and hash, and re-validating against the DB tail within the same transaction before using the cached value.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At backend/resolution_proof_service.py, line 374:
<comment>The cache stores a single global `last_hash` without any concurrency guard or DB-tail validation. In multi-worker or concurrent-request scenarios, two requests can read the same cached `prev_hash`, both commit with the same previous hash, and fork the chain. Additionally, the cache is only updated after commit, leaving a window where concurrent writers use stale data.
Consider caching both the last record ID and hash, and re-validating against the DB tail within the same transaction before using the cached value.</comment>
<file context>
@@ -368,6 +369,19 @@ def submit_evidence(
+ # 5a. Blockchain feature: calculate integrity hash for the evidence record
+ # Performance Boost: Use thread-safe cache to eliminate DB query for last hash
+ prev_hash = resolution_last_hash_cache.get("last_hash")
+ if prev_hash is None:
+ # Cache miss: Fetch only the last hash from DB
</file context>
| resolution_last_hash_cache.set(data=prev_hash, key="last_hash") | ||
|
|
||
| # Chaining logic: HMAC-SHA256(evidence_hash|gps_lat|gps_lon|prev_hash) | ||
| hash_content = f"{evidence_hash}|{gps_latitude}|{gps_longitude}|{prev_hash}" |
There was a problem hiding this comment.
P1: Float-to-string interpolation via f-string ({gps_latitude}|{gps_longitude}) can produce different representations at submission time vs. after a database round-trip (e.g., 34.0 vs 34.00000000000001). This will cause false integrity-check failures in the blockchain-verify endpoint. Use fixed-precision formatting (e.g., f"{gps_latitude:.8f}") or Decimal quantization for canonical encoding.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At backend/resolution_proof_service.py, line 382:
<comment>Float-to-string interpolation via f-string (`{gps_latitude}|{gps_longitude}`) can produce different representations at submission time vs. after a database round-trip (e.g., `34.0` vs `34.00000000000001`). This will cause false integrity-check failures in the blockchain-verify endpoint. Use fixed-precision formatting (e.g., `f"{gps_latitude:.8f}"`) or `Decimal` quantization for canonical encoding.</comment>
<file context>
@@ -368,6 +369,19 @@ def submit_evidence(
+ resolution_last_hash_cache.set(data=prev_hash, key="last_hash")
+
+ # Chaining logic: HMAC-SHA256(evidence_hash|gps_lat|gps_lon|prev_hash)
+ hash_content = f"{evidence_hash}|{gps_latitude}|{gps_longitude}|{prev_hash}"
+ integrity_hash = ResolutionProofService._sign_payload(hash_content)
+
</file context>
| hash_content = f"{evidence_hash}|{gps_latitude}|{gps_longitude}|{prev_hash}" | |
| hash_content = f"{evidence_hash}|{gps_latitude:.8f}|{gps_longitude:.8f}|{prev_hash}" |
| is_valid = False | ||
| message = "No integrity hash present; cryptographic integrity cannot be verified." | ||
| else: | ||
| is_valid = (computed_hash == evidence.integrity_hash) |
There was a problem hiding this comment.
P2: Use hmac.compare_digest() instead of == for HMAC comparison to prevent timing side-channel attacks. The codebase already uses hmac.compare_digest in _verify_signature — this comparison should be consistent.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At backend/routers/resolution_proof.py, line 262:
<comment>Use `hmac.compare_digest()` instead of `==` for HMAC comparison to prevent timing side-channel attacks. The codebase already uses `hmac.compare_digest` in `_verify_signature` — this comparison should be consistent.</comment>
<file context>
@@ -217,3 +218,62 @@ def flag_duplicate_evidence(
+ is_valid = False
+ message = "No integrity hash present; cryptographic integrity cannot be verified."
+ else:
+ is_valid = (computed_hash == evidence.integrity_hash)
+ message = (
+ "Integrity verified. This evidence record is cryptographically sealed."
</file context>
| logger.info("Added valid_from column to resolution_proof_tokens") | ||
|
|
||
| if not column_exists("resolution_proof_tokens", "valid_until"): | ||
| conn.execute(text("ALTER TABLE resolution_proof_tokens ADD COLUMN valid_until DATETIME")) |
There was a problem hiding this comment.
P2: Same issue: use TIMESTAMP instead of DATETIME for PostgreSQL compatibility.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At backend/init_db.py, line 229:
<comment>Same issue: use `TIMESTAMP` instead of `DATETIME` for PostgreSQL compatibility.</comment>
<file context>
@@ -206,6 +206,33 @@ def index_exists(table, index_name):
+ logger.info("Added valid_from column to resolution_proof_tokens")
+
+ if not column_exists("resolution_proof_tokens", "valid_until"):
+ conn.execute(text("ALTER TABLE resolution_proof_tokens ADD COLUMN valid_until DATETIME"))
+ logger.info("Added valid_until column to resolution_proof_tokens")
+
</file context>
| conn.execute(text("ALTER TABLE resolution_proof_tokens ADD COLUMN valid_until DATETIME")) | |
| conn.execute(text("ALTER TABLE resolution_proof_tokens ADD COLUMN valid_until TIMESTAMP")) |
| # Resolution Proof Tokens Table Migrations | ||
| if inspector.has_table("resolution_proof_tokens"): | ||
| if not column_exists("resolution_proof_tokens", "valid_from"): | ||
| conn.execute(text("ALTER TABLE resolution_proof_tokens ADD COLUMN valid_from DATETIME")) |
There was a problem hiding this comment.
P2: Use TIMESTAMP instead of DATETIME for cross-database compatibility. The project supports both PostgreSQL (via DATABASE_URL) and SQLite. DATETIME is a SQLite-specific type; PostgreSQL requires TIMESTAMP. Other migrations in this file use portable types.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At backend/init_db.py, line 225:
<comment>Use `TIMESTAMP` instead of `DATETIME` for cross-database compatibility. The project supports both PostgreSQL (via `DATABASE_URL`) and SQLite. `DATETIME` is a SQLite-specific type; PostgreSQL requires `TIMESTAMP`. Other migrations in this file use portable types.</comment>
<file context>
@@ -206,6 +206,33 @@ def index_exists(table, index_name):
+ # Resolution Proof Tokens Table Migrations
+ if inspector.has_table("resolution_proof_tokens"):
+ if not column_exists("resolution_proof_tokens", "valid_from"):
+ conn.execute(text("ALTER TABLE resolution_proof_tokens ADD COLUMN valid_from DATETIME"))
+ logger.info("Added valid_from column to resolution_proof_tokens")
+
</file context>
| conn.execute(text("ALTER TABLE resolution_proof_tokens ADD COLUMN valid_from DATETIME")) | |
| conn.execute(text("ALTER TABLE resolution_proof_tokens ADD COLUMN valid_from TIMESTAMP")) |
This PR introduces O(1) performance optimizations and cryptographic integrity to the Resolution Proof system.
💡 Optimizations
verify_evidencemethod was previously fetching all evidence records for a grievance into memory (.all()) before picking the last one. It now usesdb.query().count()anddb.query().order_by().first()to fetch exactly what is needed, reducing memory and DB transfer overhead.resolution_last_hash_cache(ThreadSafeCache) has been added to provide the latest hash in O(1) during record creation, avoiding redundant database lookups.📊 Expected Impact
✅ Verification
tests/test_resolution_proof.pypass.backend/init_db.py.PR created automatically by Jules for task 4910378920698676919 started by @RohanExploit
Summary by cubic
Make resolution evidence verification O(1) and add cryptographic chaining for an audit-safe trail. Adds a fast blockchain-verify endpoint and caches the last hash to speed up writes.
New Features
ResolutionEvidencewithintegrity_hashandprevious_integrity_hash.GET /api/resolution-proof/{evidence_id}/blockchain-verifyto validate a single record in O(1).resolution_last_hash_cachefor the last evidence hash.Refactors
verify_evidencenow uses.count()and.order_by(...).first()instead of loading all records.init_db.py: index onprevious_integrity_hash; addedvalid_from,valid_until, andnoncetoResolutionProofToken.Written for commit 6fa04e5. Summary will update on new commits.
Summary by CodeRabbit
New Features
Chores