feat(relay-access): gate the self-hosted iroh relay behind metadata-relay - #272
Draft
arsfeld wants to merge 13 commits into
Draft
feat(relay-access): gate the self-hosted iroh relay behind metadata-relay#272arsfeld wants to merge 13 commits into
arsfeld wants to merge 13 commits into
Conversation
…ends iroh-relay v1.0.0 names the constant X_IROH_ENDPOINT_ID but sets its value to "X-Iroh-NodeId" (src/main.rs:36, sent at :319). Upstream's own doc comment says X-Iroh-Endpoint-Id, which is where the wrong value came from. Reading the header the relay never sends made every callback arrive without an endpoint ID, answering 400, which denies 100% of relay clients. Read x-iroh-nodeid first and keep x-iroh-endpoint-id as a fallback for the day upstream corrects the constant to match its documentation.
…d scale Five defects in P2pAccess.Store, all in the same file: * do_flush passed every ETS row to one insert_all/3. At 4 bind parameters per row against exqlite's SQLITE_MAX_VARIABLE_NUMBER=32766 that caps out at 8191 rows, far below the 200k sighting cap, and the failure was swallowed by the deliberate rescue and never recovered. Chunk at 2000. * do_flush rewrote the whole table every 30 seconds regardless of what changed. Select only rows touched since the previous flush, bounded with >= so a sighting recorded in the same second as a flush is not lost. * A failed blocklist load at boot was discarded, leaving the service running with an empty blocklist and every revoked endpoint allowed until a human noticed. Retry on a timer until it loads. * list_recent/1 read only ETS, which starts empty, so after any deploy an operator saw only endpoints seen since boot. Seed sightings from the durable table at boot, capped at :p2p_max_sightings, most recent first. * That same empty start made the first post-restart flush overwrite each persisted conn_count with a counter that had restarted at zero. Seeding fixes this too: ETS resumes from the persisted value. Also documents P2P_ACCESS_BEARER_TOKENS, whose absence denies every relay client, and pins the Store's timers in the test config so they cannot interleave a background write with a sandbox-owning test.
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a new, fail-closed access-control decision point in metadata-relay for the self-hosted iroh relay, enabling the relay to gate connections via POST /p2p/access and providing durable operational tooling (sightings + blocklist) without putting database work on the relay authorization hot path.
Changes:
- Introduces
POST /p2p/accesswith bearer-token authentication and stricttext/plain"true"allow semantics, reading endpoint IDs fromx-iroh-nodeidwith anx-iroh-endpoint-idfallback. - Implements an ETS-backed
P2pAccess.Storewith capped in-memory sightings, periodic flush/prune, and a persistent blocklist with reload-at-boot behavior. - Adds migrations, runtime/test configuration, and a comprehensive test suite for endpoint normalization, routing behavior, ETS/DB interactions, and failure-path resilience.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| metadata-relay/lib/metadata_relay/router.ex | Adds POST /p2p/access endpoint and helper functions for bearer auth, endpoint ID extraction/validation, and plain-text responses. |
| metadata-relay/lib/metadata_relay/p2p_access.ex | Defines authorization policy, endpoint ID normalization, bearer validation, and recent-sightings listing. |
| metadata-relay/lib/metadata_relay/p2p_access/store.ex | Implements ETS-owned sightings/blocklist storage with boot seeding, periodic flush/prune, and failure-tolerant DB operations. |
| metadata-relay/lib/metadata_relay/p2p_access/sighting.ex | Adds Ecto schema for persisted endpoint sightings. |
| metadata-relay/lib/metadata_relay/p2p_access/block.ex | Adds Ecto schema for persisted blocked endpoints. |
| metadata-relay/lib/metadata_relay/application.ex | Supervises MetadataRelay.P2pAccess.Store as a long-lived process. |
| metadata-relay/priv/repo/migrations/20260730120000_create_p2p_access_tables.exs | Creates SQLite tables for sightings and blocked endpoints (with index on last_seen). |
| metadata-relay/config/config.exs | Introduces default configuration for bearer tokens, sighting cap, flush/prune cadence, and retention window. |
| metadata-relay/config/runtime.exs | Loads P2P_ACCESS_BEARER_TOKENS from env, normalizes to a token list, and configures runtime access. |
| metadata-relay/config/test.exs | Pins Store timers far into the future to avoid sandbox ownership issues during tests; configures a default test bearer. |
| metadata-relay/test/metadata_relay/p2p_access_test.exs | Unit tests for normalization, authorization decisions, bearer validation, blocking/unblocking, and list_recent behavior. |
| metadata-relay/test/metadata_relay/p2p_access/router_test.exs | Router-level tests verifying bearer gating, header parsing (including fallback), response format, and sighting recording. |
| metadata-relay/test/metadata_relay/p2p_access/store_test.exs | Store tests covering ETS cap behavior, flush/prune persistence and chunking, boot reload/seed behavior, and DB-failure resilience. |
| metadata-relay/test/metadata_relay/p2p_access/schema_test.exs | Schema round-trip tests for Sighting and Block. |
| metadata-relay/README.md | Documents the new P2P_ACCESS_BEARER_TOKENS environment variable and its behavior. |
Comment on lines
+10
to
+12
| Reads and writes here are on the relay authorization hot path, so every | ||
| public function in this module must be ETS-only. Database work happens at | ||
| boot and on timers inside the GenServer, never inline with a request. |
Collaborator
Author
There was a problem hiding this comment.
Fixed in b88dafb. The claim was true when it was written, then quietly stopped being true as the blocklist and the flush/prune timers landed on top of it. The moduledoc now scopes the ETS-only guarantee to record_sighting/1 and blocked?/1, the two functions actually on the relay hot path, and states where the rest of the database work happens (boot seeding, timers, admin actions).
Comment on lines
+362
to
+368
| test "prune degrades to a rescued {:ok, count} and keeps the Store alive when the database delete fails" do | ||
| stale = endpoint_id(46) | ||
| now = System.system_time(:second) | ||
|
|
||
| :ets.insert(:p2p_sightings, {stale, now - 100, now - 100, 1}) | ||
| Application.put_env(:metadata_relay, :p2p_retention_seconds, 50) | ||
|
|
Collaborator
Author
There was a problem hiding this comment.
Fixed in b88dafb. This was a known residual, called out under "Known residual" in the PR description, so good catch confirming it independently. Both database-failure tests now call restore_sandbox_on_exit/0 before the checkin, and I removed the redundant trailing restore from the flush test too, since nothing after the checkin in either test needs the connection. Verified across five seeds: 235 tests, 0 failures.
…x from on_exit The moduledoc claimed every public function was ETS-only, which stopped being true once the blocklist and the flush/prune timers landed. Narrow the guarantee to the two hot-path functions it actually applies to and say plainly where the database work happens. The prune database-failure test restored the sandbox with trailing statements, so a failing assertion would skip the restore and poison later tests. Both failure-path tests now restore from on_exit instead.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Makes our self-hosted iroh relay (
cae1-1.relay.mydia.dev) ask metadata-relay whether to accept each connecting endpoint, instead of running wide open to anyone who reads the URL out of this repository.The relay's stock
access.httpmode POSTs the connecting endpoint's ID to a URL we choose and grants access only on a200whose body is the exact texttrue. That endpoint ID is cryptographically proven by the relay handshake, so metadata-relay can make a real policy decision without the client presenting any credential.This is phase 1 of the design and ships inert. Nothing calls the new endpoint until the relay configmap changes, which is not in this PR.
What's here
POST /p2p/accesson metadata-relay, authenticated by a shared bearer the relay presents (P2P_ACCESS_BEARER_TOKENS, comma-separated so tokens can be rotated; unset denies everything)MetadataRelay.P2pAccess.block/2,unblock/1,list_recent/1The request path is ETS-only by design and verified as such: no database, Redis, network call, or
GenServer.callbetween the HTTP request and the allow/deny decision. This matters because the callback is fail-closed — if it is slow or errors, the relay refuses the client.235 tests, 0 failures.
What this deliberately does not do
It does not authenticate "is this a real Mydia instance." Mydia is open source and self-hosted with no accounts, so any credential we ship is extractable. What it delivers is attribution, quotas, and revocation: we will know who is using the relay, usage is bounded, and a specific abuser can be cut off.
Caught in review, worth knowing
The implementation originally read an
X-Iroh-Endpoint-Idheader. The relay does not send that. Iniroh-relayv1.0.0,src/main.rs:36definesconst X_IROH_ENDPOINT_ID: &str = "X-Iroh-NodeId"while its own doc comment at:170advertisesX-Iroh-Endpoint-Id— upstream's documentation contradicts upstream's code. Had this shipped, enabling access control would have denied 100% of clients. It now readsx-iroh-nodeidwithx-iroh-endpoint-idas a deliberate fallback; please do not remove the fallback.Also found upstream and relevant to rollout:
iroh-relayv1.0.0 builds its HTTP access client with no request or connect timeout (src/main.rs:221). A slow metadata-relay therefore hangs relay handshakes rather than denying them quickly, which is a worse failure mode than an outright outage.Not included, and required before this does anything
Two follow-up steps need a human, docker, and cluster access:
[access.http]TOML parses against the actual binary, and — the load-bearing one — confirm an iroh client whose relay rejects it actually re-homes to one of n0's fallback relays. The entire safety story rests on that last one and it is still unverified.access = "everyone"is the escape hatch.Review follow-ups
Copilot raised two points, both fixed in b88dafb:
Storemoduledoc claimed every public function was ETS-only. That was true when written and stopped being true once the blocklist and the flush/prune timers landed on top of it. The guarantee is now scoped to the two functions actually on the hot path.on_exit. This was the residual originally listed here, so it is resolved.