Scaffolding: Federation sync app creation#298
Conversation
klpoland
added
feature
klpoland
changed the title
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
|
I'll add more comments after those are addressed to reduce the noise |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 4 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 7bcbcba. Configure here.
| site_name=doc.site_name, | ||
| asset=doc, | ||
| asset_type=asset_type, | ||
| ) |
There was a problem hiding this comment.
Bootstrap trusts export site names
Medium Severity
bootstrap_gateway_exports indexes each export document using doc.site_name without checking it matches the peer (or local site) being pulled. A misconfigured or hostile export can write into another site’s site:uuid keys, including overwriting this site’s own federated documents.
Reviewed by Cursor Bugbot for commit 7bcbcba. Configure here.
| hello = hello.model_copy(update={"timestamp": datetime.now(UTC)}) | ||
|
|
||
| _peer_registry(request).register(hello) | ||
| return {"status": "registered", "site_name": hello.site_name} |
There was a problem hiding this comment.
Unauthenticated hello rewrites peer URLs
Medium Severity
site-hello accepts any caller that supplies a configured peer site_name and stores their sync_service_url with no authentication. peer_for_outbound then prefers that URL for asset webhooks, so a reachable sync port lets an attacker redirect outbound federation POSTs to an arbitrary host.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 7bcbcba. Configure here.
| id=_id, | ||
| body=body, | ||
| refresh="wait_for", | ||
| ) |
There was a problem hiding this comment.
Sync OpenSearch blocks async handlers
Medium Severity
apply_asset_event performs synchronous OpenSearch index calls with refresh="wait_for" directly from async webhook handlers and bootstrap. Unlike federated reads, writes are not offloaded with asyncio.to_thread, so each peer update can stall the event loop and delay Redis subscription and health responses.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 7bcbcba. Configure here.
| return self._peers.get(site_name) | ||
|
|
||
| def known_site_names(self) -> set[str]: | ||
| return set(self._peers.keys()) |
There was a problem hiding this comment.
Unused peer registry tracking fields
Low Severity
PeerRegistry stores _last_seen on every register and exposes known_site_names, but nothing in the app or tests reads either. That dead state suggests incomplete registry behavior and adds noise for future maintainers.
Reviewed by Cursor Bugbot for commit 7bcbcba. Configure here.
2 participants
In this PR:
federation/(v1) application with:services:bootstrap.py: for initialization of federated data from home site and peers (including when new peer is added)fed_index.py: for handling (external peer) asset indexing on webhook eventslocal_events.py: for handling redis dispatches to fetch local datapeer registry.py: for registering new peers and holding site information from site hellospeer_sync.py: for packaging and sending asset data to peer sites (through their webhook routes)routes:health.py: (stub for now) for pinging site health (is connection established?)webhooks.py: for receiving payloads FROM peers to index into LOCAL federated asset indices as well as asite-hellohook to confirm federation between peersschemasandmodels: for type checking and providing data structures to federation configs, events, and documents (how asset docs are represented byFederatedDatasetDoc,FederatedCaptureDocmay warrant further consideration re: anticipating schema flexibility)mainapp that subscribes to redis events for picking up signals from gateway (handled in PR Scaffolding: Federation gateway setup #299)Note
Medium Risk
New cross-site data path (OpenSearch writes, unauthenticated peer webhooks gated only by config allowlists) and startup bootstrap against gateway exports; mitigated by strict schema validation and origin checks but worth careful ops review.
Overview
Adds a new
federation/package: a FastAPI sync service that keeps fed-datasets / fed-captures OpenSearch indices in sync across SDS sites.On startup it can bootstrap from local and peer gateway
/federation/export/APIs, thensite-hellopeers. A Redis pub/sub subscriber (site channelfederation:events:{site}) loads the local asset from OpenSearch and POSTs asset-updated webhooks to configured peers; inbound webhooks index peer assets with allowlistedsite_nameand stale-event skipping./sync/healthreports Redis, OpenSearch, and subscriber readiness.Also adds Docker/compose, justfile, dev mTLS cert scripts, example
federation.toml, and extends pre-commit (federation-specific ruff, deptry, pyrefly) plus a broad pytest suite (mesh, bootstrap, webhooks).Reviewed by Cursor Bugbot for commit 7bcbcba. Bugbot is set up for automated code reviews on this repo. Configure here.