Skip to content

fix: Carry relay auth material into the per-space network config - #148

Closed
ThetaSinner wants to merge 1 commit into
mainfrom
fix/relay-auth-material-main
Closed

fix: Carry relay auth material into the per-space network config#148
ThetaSinner wants to merge 1 commit into
mainfrom
fix/relay-auth-material-main

Conversation

@ThetaSinner

Copy link
Copy Markdown
Member

A conductor using an authenticated relay loses relay access the moment its first
cell is created, and nothing in the auth path reports a problem.

Symptom

Authenticating for relay access server_url=https://…/
Keeping iroh endpoint key registered … iroh_key=M6ZTsg…
Iroh relay keepalive succeeded
Relay authentication complete, proceeding to insert relay
…::app_status_impls: Creating a cell cell_id=CellId(DnaHash(…), …)
do_insert_relay: relay added, local URL constructed
iroh::socket::transports::relay::actor: Failed to connect to relay server:
    unable to connect: The relay denied our authentication (not authorized)

The HTTP auth never fails. The keepalive keeps succeeding every two minutes for
as long as the process runs, so the node holds a valid token and a live allowlist
entry while being refused at the socket. The only symptom is that it reaches no
peers, with every service reporting healthy.

Cause

base64_auth_material_relay seeds the transport at startup only. The config
NetworkConfig::to_k2_config builds carries irohTransport.relayUrl but no
material, and IrohTransport::configure_for_space re-inserts the relay from that
config when a space is created:

let relay_url = per_space.as_ref().and_then(|c| c.relay_url.clone());
let auth_material = per_space.as_ref()
    .and_then(|c| c.auth_material_relay_base64.as_ref())   // never setif let Some(url) = relay_url {do_insert_relay(endpoint, url, auth_material)}

relay_url is always present and auth_material never is, so the authenticated
relay is replaced by a tokenless one. TokenAccess::on_connect gates both of its
allow-paths on a token being present, so the recovery allowlist cannot admit it
either.

A node with a public address never notices — it connects directly and never
exercises the relay socket. It only bites peers behind NAT, which is why a fleet
of public nodes can look entirely healthy while every desktop client fails.

Fix

Write the material into network.advanced as well, where kitsune2 reads it per
space. to_k2_config merges into advanced rather than replacing it, so the
value survives alongside the URL it writes — and a caller's own advanced settings
(a testnet build sets relayAllowPlainText there) survive too, which the second
test pins.

Both injection sites are covered: the boot path and restart_with_hc_auth.
Patching only the first works on a cold start and fails after an in-app restart.

Verification

Two unit tests, plus an end-to-end check: a NAT'd desktop client that previously
never reached a peer now joins a production network behind an authenticated
kitsune2_bootstrap_srv and syncs. A public peer passes either way, so it is not
a useful test of this.

Two things worth a second opinion

  1. This may belong upstream instead. kitsune2 documents
    auth_material_relay_base64 as "Ignored in the global config", which forces
    every consumer to duplicate the material per space. If that asymmetry is not
    deliberate, the better fix is for holochain's to_k2_config to carry it — or
    for kitsune2 to fall back to the global material when a per-space override
    supplies none. This change is correct regardless, but it is a workaround for
    an awkward contract.
  2. It puts a credential in the logs. Holochain redacts
    auth_material_relay in its debug output but prints network_config in full,
    so the material now appears in clear text in the boot line. It is effectively a
    bearer credential for the relay. Worth pairing with redaction wherever this
    lands.

`base64_auth_material_relay` only seeds the transport at startup. The config
`NetworkConfig::to_k2_config` builds carries `irohTransport.relayUrl` but no
material, and kitsune2 re-inserts the relay from that config when a space is
created - tokenless, which an authenticated relay refuses.

The node keeps authenticating and keeps its relay allowlist entry alive, so
nothing in the auth path looks wrong; it simply reaches no peers from the moment
its first cell is created. A node with a public address never notices, because it
connects directly and never exercises the relay socket.

Write the material into `network.advanced` as well, where kitsune2 reads it per
space. `to_k2_config` merges into `advanced` rather than replacing it, so the
value survives alongside the URL it writes, and a caller's own advanced settings
survive too.

Both injection sites are covered: the boot path and `restart_with_hc_auth`.
Patching only the first works on a cold start and fails after an in-app restart.
@ThetaSinner
ThetaSinner had a problem deploying to Android Service Runtime Release August 28, 2026 10:00 — with GitHub Actions Failure
@ThetaSinner
ThetaSinner had a problem deploying to Android Service Runtime Release August 28, 2026 10:00 — with GitHub Actions Failure
@ThetaSinner
ThetaSinner had a problem deploying to Android Service Runtime Release August 28, 2026 10:00 — with GitHub Actions Failure
@cocogitto-bot

cocogitto-bot Bot commented Aug 28, 2026

Copy link
Copy Markdown

✔️ d386016 - Conventional commits check succeeded.

@coderabbitai

coderabbitai Bot commented Aug 28, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: b497c73e-0504-44b5-bacc-7ac8230136ea

📥 Commits

Reviewing files that changed from the base of the PR and between a7b5bb1 and d386016.

📒 Files selected for processing (1)
  • crates/runtime/src/runtime.rs

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


Walkthrough

The runtime adds a helper that stores relay authentication material at network.advanced.irohTransport.authMaterialRelayBase64. The helper preserves existing advanced settings and logs a warning for invalid JSON structures. Boot configuration and restart authentication flows now invoke the helper when relay authentication material is available. Tests verify the target JSON location and preservation of existing settings.

Suggested reviewers: zippy, zo-el

Merge Risk: 🔵 Low · up to d3860

The change restores authenticated relay connectivity during boot and restart, but it carries a bounded merge-readiness risk: the relay bearer credential is duplicated in generic configuration where redaction is unverified, and incompatible configuration shapes could still leave authenticated nodes unable to use the relay.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: carrying relay authentication material into the per-space network configuration.
Description check ✅ Passed The description is detailed and directly explains the symptom, cause, fix, affected paths, tests, end-to-end verification, and follow-up concerns. It does not include the template's explicit Summary o…
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 1 files.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Description check

Explanation

The description is detailed and directly explains the symptom, cause, fix, affected paths, tests, end-to-end verification, and follow-up concerns. It does not include the template's explicit Summary or TODO checklist sections, but the required change information is substantially present.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/relay-auth-material-main

Warning

Some tools did not complete. Review the errors below.

🔧 Clippy (1.97.1)

Clippy execution timed out


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@ThetaSinner

Copy link
Copy Markdown
Member Author

Closing: this fixes the symptom from the outside.

The per-space configuration hook runs even when no override was requested, so the transport reconfigures the relay it already authenticated to and loses the token. Having every consumer write the material into per-space settings papers over that, and copies a bearer credential into a field that gets logged in clear text.

Fixing it in kitsune2 instead, where the hook should not be firing for a space that overrides nothing.

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