Repository navigation
cmux-tui: broker-registered iroh transport sidecar (stage 1) - #9524
Closed
azooz2003-bit wants to merge 3 commits into
Closed
azooz2003-bit wants to merge 3 commits into
azooz2003-bit wants to merge 3 commits into
Conversation
Stage 1 of the iroh TUI transport program (docs/iroh-tui-transport-stage1.md): a sidecar that makes a cmux-tui session reachable by dialing its EndpointID alone through the cmux account device registry and the managed relay fleet. - listen: registers the device's (user, deviceId, tag) binding slot with the trust broker, binds a Minimal-preset endpoint on the verified relay catalog (never n0 defaults), and admits only connections whose first stream carries a broker-signed cmux/tui/1 pair grant naming this exact acceptor and the TLS-authenticated initiator. Admitted streams bridge byte-for-byte to the session Unix socket (unchanged protocol v10 JSON-lines, relay-stdio authority model). Broker state revalidates every 30s; grant expiry closes idle sessions; relay credentials rotate lazily via remove/insert_relay. - enroll: exchanges a one-use provisioning token for a Stack session credential (POST /api/devices/iroh/enroll), mints the persistent Ed25519 endpoint key + deviceId, and registers the slot; restarts re-register the same slot idempotently. - provider: machine-provider v1 over stdio for cmux-tui --machine-provider-command. Control owns discovery, pair grants, and the single iroh endpoint; per-ticket stream processes rendezvous over a private unix socket so no second endpoint binds the same key. Broker counterpart (linux platform, TUI grant profile, enrollment routes) is #9515. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Dockerfile builds a headless cmux-tui + cmux-tui-iroh image (zig for libghostty-vt; a bind-mounted plain resolv.conf works around zig's ResolvConfParseFailed on Docker-generated resolver files). run-demo.sh drives the stage-1 acceptance: zero-published-port container enrolls with a one-use token, registers, and listens; the Mac side enrolls its own identity and attaches purely by EndpointID through the broker registry (attach-once.py exercises the exact machine-provider v1 + admission + protocol v10 seams the TUI uses); detach/reattach and a container restart re-registering the same slot are asserted. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
📝 WalkthroughWalkthroughAdds the ChangesIroh transport sidecar
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant Provider
participant Broker
participant Listener
participant SessionSocket
Client->>Provider: Request machine snapshot and open_machine
Provider->>Broker: Discover machine and create pair grant
Provider->>Listener: Dial endpoint and submit grant
Listener->>SessionSocket: Open local session
Listener-->>Provider: Return admission acknowledgment
Provider-->>Client: Return transport stream
Client->>Provider: Send session commands
Provider<<->>SessionSocket: Forward session bytes
Possibly related PRs
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (3 errors, 1 warning)
✅ Passed checks (21 passed)
✨ Finishing Touches 💡 1📝 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 19
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cmux-tui/crates/cmux-tui-iroh/src/broker.rs`:
- Around line 39-56: Validate the broker and stack origins in
BrokerConfig::resolve so credential-bearing requests only use HTTPS, rejecting
unsafe --broker, CMUX_TUI_IROH_BROKER, or CMUX_TUI_IROH_STACK_BASE values before
requests proceed. In cmux-tui/crates/cmux-tui-iroh/src/broker.rs lines 39-56,
enforce this origin validation; in cmux-tui/crates/cmux-tui-iroh/src/enroll.rs
lines 37-40, apply the validation before the enrollment POST and disable
redirects on the trusted reqwest client used for authenticated broker calls.
In `@cmux-tui/crates/cmux-tui-iroh/src/endpoint.rs`:
- Around line 85-93: Update the relay-rotation loop around fresh_relay_token to
track consecutive rotation failures and apply increasing backoff before retrying
after an error. Ensure the retry delay keeps mint attempts within the documented
limit of 3 per 10 minutes, while resetting the failure counter and normal retry
behavior after a successful token rotation.
In `@cmux-tui/crates/cmux-tui-iroh/src/files.rs`:
- Around line 103-107: Update the successful rename flow around fs::rename to
open the parent directory of path and call sync_all after the replacement
completes. Propagate any directory-open or sync_all failure with appropriate
context, while preserving temporary-file cleanup and the existing rename error
handling.
In `@cmux-tui/crates/cmux-tui-iroh/src/listen.rs`:
- Around line 221-233: Update the watchdog reader setup in the tokio::spawn
block to wrap admission_recv with take(ADMISSION_MAX_BYTES as u64) before
constructing BufReader, so read_line is bounded during buffering; retain the
existing sink length check and connection-close behavior.
- Around line 235-241: Update the expiry task spawned near expiry_connection to
select between the grant-expiry sleep and the connection’s close event, exiting
immediately when the connection closes and only calling close with “grant
expired” when the timer wins. Preserve the existing expiry delay and
connection-close behavior.
In `@cmux-tui/crates/cmux-tui-iroh/src/main.rs`:
- Around line 88-105: Update Flags::parse to recognize a bare "--" as the
end-of-flags separator: stop interpreting subsequent arguments as flags and
collect the separator’s trailing arguments in positional. Preserve existing
value-flag, boolean-flag, unknown-flag, and positional handling before the
separator, including provider commands such as control.
In `@cmux-tui/crates/cmux-tui-iroh/src/provider.rs`:
- Around line 143-160: Update the rendezvous accept loop around
rendezvous.accept() so recoverable accept errors are logged and the loop
continues instead of breaking. Preserve termination for unrecoverable errors by
distinguishing the error cases appropriately, ensuring the spawned transport
listener remains available for later stream connections.
- Around line 465-475: Update ensure_connection to return
anyhow::Result<String>, yielding the existing surviving connection id on its
early-return path and the inserted connection_id after creation. In
open_machine, use the returned id directly from ensure_connection and remove the
state_guard lookup, map search, and expect panic.
- Around line 640-664: In the handshake setup block, clone the machine
connection or required connection handle while the ControlState guard is held,
then exit the state.lock().await scope before calling open_bi(). Await
machine.connection.open_bi() only after the mutex guard has been dropped, while
preserving ticket validation, removal, and one-use semantics inside the critical
section.
- Around line 740-761: Update run_stream_blocking to collect the handshake in a
Vec<u8> and append each received byte unchanged, while retaining the
maximum-size validation. Parse the handshake with serde_json::from_slice(&line),
and forward the exact bytes via socket.write_all(&line) followed by the existing
separate newline write.
In `@cmux-tui/crates/cmux-tui-iroh/src/relays.rs`:
- Around line 76-83: Update the canonical path in
embedded_catalog_matches_committed_source_of_truth to resolve to the committed
repository catalog under config/iroh/managed-relay-catalog.json. Ensure the test
reads that source-of-truth file rather than an in-crate path, so missing-path
handling cannot silently skip drift validation.
In `@cmux-tui/scripts/iroh-docker-demo/attach-once.py`:
- Around line 63-75: Replace the blocking stdout.readline calls in
ControlClient.request and SessionStream.command with one shared bounded-read
helper that enforces the remaining timeout and raises the caller-specific
timeout error (“{method} timed out” or “{cmd} timed out”) when no complete line
arrives. Apply the helper at cmux-tui/scripts/iroh-docker-demo/attach-once.py
lines 63-75 and 123-134; both sites should use the same timeout-aware behavior.
- Around line 218-219: Implement actual execution for the --send-command path in
the attach-once harness by routing the command to the attached surface before
continuing, rather than only logging that it was skipped. Update the relevant
command-flow logic around args.send_command and ensure run-demo.sh’s invocation
exercises the command successfully; if wiring is not feasible, remove the flag
usage and document the missing stage-1 capability in
docs/iroh-tui-transport-stage1.md.
- Line 184: Update the machine-list formatting expression in the logging path to
treat each machine’s subtitle as optional, matching the handling at line 190.
Use the same fallback behavior for missing subtitle keys so logging continues
and tag matching can run.
- Around line 186-193: Update the target selection logic around the machines
iterator to match args.machine_tag against each machine descriptor’s identifier
or binding-id field rather than display_name or subtitle. Detect multiple
matching rows and fail instead of selecting the first match, while preserving
the existing no-match handling.
In `@cmux-tui/scripts/iroh-docker-demo/entrypoint.sh`:
- Around line 26-32: Replace the fixed sleep-based polling in entrypoint.sh with
an explicit readiness signal from the headless cmux-tui process, and only start
cmux-tui-iroh listen after that signal confirms the session socket is available.
If readiness is not observed before the timeout, exit nonzero and do not invoke
the listener.
In `@cmux-tui/scripts/iroh-docker-demo/mint-enrollment-token.mjs`:
- Around line 59-69: Validate the parsed sign-in response in the mint-enrollment
flow before constructing the request, requiring a usable session.access_token
(and the existing refresh token if needed). If the token is missing or the
response shape is invalid, fail immediately with a clear sign-in error instead
of sending an authorization header containing an undefined value; keep the
valid-session mint request unchanged.
In `@cmux-tui/scripts/iroh-docker-demo/run-demo.sh`:
- Around line 111-112: Remove the python3-based assignment from the binding ID
lookup and retain a single grep/sed read path in the BINDING_BEFORE
initialization. Preserve the existing fallback behavior while eliminating the
unavailable runtime dependency.
In `@docs/iroh-tui-transport-stage1.md`:
- Line 28: Update the Minimal preset documentation to identify
cmux-tui/crates/cmux-tui-iroh/relay-catalog.json as the embedded runtime
catalog, noting it is compiled from config/iroh/managed-relay-catalog.json.
Revise the crate documentation in relays.rs to describe the same
include_str!-based embedded catalog and canonical-source relationship.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: a632518e-f158-4717-988d-03db50d3444b
⛔ Files ignored due to path filters (1)
cmux-tui/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (23)
.dockerignorecmux-tui/Cargo.tomlcmux-tui/crates/cmux-tui-iroh/Cargo.tomlcmux-tui/crates/cmux-tui-iroh/relay-catalog.jsoncmux-tui/crates/cmux-tui-iroh/src/broker.rscmux-tui/crates/cmux-tui-iroh/src/endpoint.rscmux-tui/crates/cmux-tui-iroh/src/enroll.rscmux-tui/crates/cmux-tui-iroh/src/files.rscmux-tui/crates/cmux-tui-iroh/src/grant.rscmux-tui/crates/cmux-tui-iroh/src/identity.rscmux-tui/crates/cmux-tui-iroh/src/listen.rscmux-tui/crates/cmux-tui-iroh/src/main.rscmux-tui/crates/cmux-tui-iroh/src/provider.rscmux-tui/crates/cmux-tui-iroh/src/relays.rscmux-tui/crates/cmux-tui-iroh/src/timefmt.rscmux-tui/scripts/iroh-docker-demo/Dockerfilecmux-tui/scripts/iroh-docker-demo/attach-once.pycmux-tui/scripts/iroh-docker-demo/build-resolv.confcmux-tui/scripts/iroh-docker-demo/entrypoint.shcmux-tui/scripts/iroh-docker-demo/mint-enrollment-token.mjscmux-tui/scripts/iroh-docker-demo/run-demo.shcmux-tui/spec/transports.mddocs/iroh-tui-transport-stage1.md
Broker client: enforce https (or loopback http) for broker and Stack origins and disable redirect following so bearer headers cannot leak to a redirect target. Relay rotation: exponential cooldown after failed mints (2^n minutes, capped 32) keeps a persistently down relay inside the 3-per-10-minute quota instead of burning it every 30s. Identity files: fsync the parent directory after the atomic rename so power loss right after enrollment cannot lose the registration authority. Listener: bound the liveness-channel read during the read (take + set_limit), and cancel the grant-expiry sleeper when its connection closes. Flags: treat -- as a separator. Provider: survive transient rendezvous accept errors, return the connection id from ensure_connection (drops the expect lookup), open session streams outside the state lock so a slow machine cannot stall the control loop, and collect the stream handshake as raw bytes so non-ASCII frames are not re-encoded. Demo harness: bounded line reads that honor timeouts (reader threads), select machines by descriptor id or exact registered display name instead of substring text, wire --send-command through protocol v10 send + read-screen with a quote-split marker, gate the container listener on session-socket readiness and fail closed, validate Stack sign-in fields in the mint helper, drop the dead python3 branch, and fix the runtime image volume ownership (chown the state dir before VOLUME). Docs: name the embedded relay-catalog copy and its canonical source. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
cmux-tui/crates/cmux-tui-iroh/src/provider.rs (1)
154-160: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftDo not retry every accept error with a fixed sleep.
A persistent accept error makes this detached task log every 200 ms indefinitely. The control loop remains active and can issue tickets although no rendezvous stream can connect.
Retry only recoverable errors such as
InterruptedorConnectionAborted. Propagate terminal errors torun_control. If resource exhaustion requires backoff, use a bounded cancellation-aware retry abstraction.As per coding guidelines and path instructions, production socket recovery must not use fixed wall-clock waits as synchronization.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmux-tui/crates/cmux-tui-iroh/src/provider.rs` around lines 154 - 160, Update the accept-error handling in the rendezvous accept loop around the Err(error) branch to retry only recoverable errors such as Interrupted or ConnectionAborted, and propagate terminal errors to run_control instead of continuing indefinitely. Remove the unconditional fixed 200ms sleep; if resource-exhaustion backoff is needed, use a bounded cancellation-aware retry mechanism.Sources: Coding guidelines, Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cmux-tui/crates/cmux-tui-iroh/src/broker.rs`:
- Around line 77-83: Update the URL validation in the broker client setup around
the loopback detection: parse the URL with reqwest’s URL representation, reject
any username or password, and permit HTTP only when the parsed host is a
loopback address; remove the host.docker.internal exception. Ensure bracketed
IPv6 localhost (http://[::1]) remains allowed, reject userinfo such as
http://localhost:80@attacker.example, and add regressions covering both cases.
- Line 87: Update the validation error in the broker URL parsing flow to omit
the raw URL value. Keep the configuration name and required
HTTPS-or-loopback-HTTP guidance in the message, but remove the `{url:?}`
interpolation so query strings and userinfo cannot reach CLI output.
In `@cmux-tui/scripts/iroh-docker-demo/attach-once.py`:
- Around line 134-136: Replace the raw response interpolation in the
transport-handshake rejection raised by the reader logic in attach-once.py with
a stable product-level error message; if diagnostics are needed, log only a
bounded internal code rather than provider fields. In
cmux-tui/scripts/iroh-docker-demo/mint-enrollment-token.mjs lines 60-62, apply
the same redaction behavior to the corresponding handshake rejection site.
---
Duplicate comments:
In `@cmux-tui/crates/cmux-tui-iroh/src/provider.rs`:
- Around line 154-160: Update the accept-error handling in the rendezvous accept
loop around the Err(error) branch to retry only recoverable errors such as
Interrupted or ConnectionAborted, and propagate terminal errors to run_control
instead of continuing indefinitely. Remove the unconditional fixed 200ms sleep;
if resource-exhaustion backoff is needed, use a bounded cancellation-aware retry
mechanism.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 0f4af54c-668a-4b4d-99af-0b12b96c13b7
📒 Files selected for processing (14)
cmux-tui/crates/cmux-tui-iroh/src/broker.rscmux-tui/crates/cmux-tui-iroh/src/endpoint.rscmux-tui/crates/cmux-tui-iroh/src/enroll.rscmux-tui/crates/cmux-tui-iroh/src/files.rscmux-tui/crates/cmux-tui-iroh/src/listen.rscmux-tui/crates/cmux-tui-iroh/src/main.rscmux-tui/crates/cmux-tui-iroh/src/provider.rscmux-tui/crates/cmux-tui-iroh/src/relays.rscmux-tui/scripts/iroh-docker-demo/Dockerfilecmux-tui/scripts/iroh-docker-demo/attach-once.pycmux-tui/scripts/iroh-docker-demo/entrypoint.shcmux-tui/scripts/iroh-docker-demo/mint-enrollment-token.mjscmux-tui/scripts/iroh-docker-demo/run-demo.shdocs/iroh-tui-transport-stage1.md
Comment on lines
+77
to
+83
| let loopback = url.strip_prefix("http://").is_some_and(|rest| { | ||
| let host = rest.split(['/', ':']).next().unwrap_or(""); | ||
| host == "localhost" | ||
| || host == "127.0.0.1" | ||
| || host == "[::1]" | ||
| || host == "host.docker.internal" | ||
| }); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Sensitive Data Exposure (CWE-319): Cleartext Transmission of Sensitive Information
Reachability: External
Reachability path
● Entry
cmux-tui/crates/cmux-tui-iroh/src/provider.rs:49
run
│
▼
● Hop
cmux-tui/crates/cmux-tui-iroh/src/enroll.rs:24
run
│
▼
● Hop
cmux-tui/crates/cmux-tui-iroh/src/main.rs
│
▼
● Hop
cmux-tui/crates/cmux-tui-iroh/src/listen.rs:63
run: Register (heartbeat) this device's slot and refresh the verification keys.
│
▼
● Hop
cmux-tui/crates/cmux-tui-iroh/src/endpoint.rs:65
spawn_relay_maintenance: Exponential backoff between rotation attempts keeps a persistently
│
▼
● Sink
cmux-tui/crates/cmux-tui-iroh/src/broker.rs
Validate the parsed authority before allowing HTTP.
split(['/', ':']) reads userinfo as the host. http://localhost:80@attacker.example passes this loopback branch, but the request targets attacker.example. BrokerClient::enroll then sends the enrollment token, and authenticated calls send bearer and refresh credentials over cleartext. An attacker needs to control a configured broker or Stack origin. The host.docker.internal exception also permits a non-TLS hop.
Parse the URL, reject userinfo, and allow HTTP only for parsed loopback endpoints. Add regressions for this input and http://[::1]. Reqwest’s URL representation separates userinfo from host. (docs.rs)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@cmux-tui/crates/cmux-tui-iroh/src/broker.rs` around lines 77 - 83, Update the
URL validation in the broker client setup around the loopback detection: parse
the URL with reqwest’s URL representation, reject any username or password, and
permit HTTP only when the parsed host is a loopback address; remove the
host.docker.internal exception. Ensure bracketed IPv6 localhost (http://[::1])
remains allowed, reject userinfo such as http://localhost:80@attacker.example,
and add regressions covering both cases.
| if loopback { | ||
| return Ok(()); | ||
| } | ||
| bail!("{what} {url:?} must be https (or loopback http for local development)"); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== broker.rs relevant section =="
sed -n '1,140p' cmux-tui/crates/cmux-tui-iroh/src/broker.rs | cat -n
echo
echo "== CLI args around broker option =="
rg -n "broker|BrokerConfig|struct .*Args|clap|Parser|ValueEnum" cmux-tui/crates/cmux-tui-iroh/src/main.rs cmux-tui/crates/cmux-tui-iroh/src -g '*.rs'
echo
echo "== anyhow and error contexts/usages =="
rg -n "Bail!"|rg -n "context\\(|format!\\(|print|println|eprintln|bail!(" cmux-tui/crates/cmux-tui-iroh/src/main.rs cmux-tui/crates/cmux-tui-iroh/src -g '*.rs' | head -n 200Repository: manaflow-ai/cmux
Length of output: 16997
🌐 Web query:
Rust std::string::String Debug {:?} includes quotes quotes escaped
💡 Result:
In Rust, the Debug trait implementation for String (and str) is specifically designed to provide a programmer-facing, unambiguous representation of the string's content [1][2][3]. When you use the {:?} format specifier, the resulting output automatically includes double quotes surrounding the string and escapes any non-printable characters or control characters [1][4]. Key aspects of this behavior include: 1. Quotes: The output is wrapped in double quotes (e.g., "hello") to clearly delimit the start and end of the string value [1][3]. 2. Escaping: Non-printable characters, control characters, and certain Unicode codepoints are escaped [1][4]. For example, a newline character becomes \n, a tab becomes \t, and double quotes within the string are escaped as \" [4][3]. 3. Purpose: This approach is intended for debugging [2][3]. By including quotes and escaping hidden characters, it allows developers to distinguish between empty strings, strings with whitespace, and strings containing special characters, which would otherwise be difficult or impossible to see with the Display trait ({}) [1][3]. This behavior is mandated by the Debug implementation for str, which String uses via dereferencing [5][2]. The escaping logic is specifically designed to make the string's content explicit and easy to inspect during development [1][4].
Citations:
- 1: https://rust-lang.github.io/rfcs/0565-show-string-guidelines.html
- 2: https://doc.rust-lang.org/stable/std/fmt/trait.Debug.html
- 3: https://doc.rust-lang.org/stable/std/fmt/index.html
- 4: Escape fewer Unicode codepoints in
Debugimpl ofstrrust-lang/rust#34485 - 5: https://github.com/rust-lang/rust/blob/a85ec7c2c005c1c2f85050151706c6301720760a/src/liballoc/string.rs
Sensitive Data Exposure (CWE-532): Insertion of Sensitive Information into Log File
Reachability: External
Reachability path
● Entry
cmux-tui/crates/cmux-tui-iroh/src/provider.rs:49
run
│
▼
● Hop
cmux-tui/crates/cmux-tui-iroh/src/enroll.rs:24
run
│
▼
● Hop
cmux-tui/crates/cmux-tui-iroh/src/main.rs
│
▼
● Hop
cmux-tui/crates/cmux-tui-iroh/src/listen.rs:63
run: Register (heartbeat) this device's slot and refresh the verification keys.
│
▼
● Hop
cmux-tui/crates/cmux-tui-iroh/src/endpoint.rs:65
spawn_relay_maintenance: Exponential backoff between rotation attempts keeps a persistently
│
▼
● Sink
cmux-tui/crates/cmux-tui-iroh/src/broker.rs
Redact the rejected broker URL from CLI errors.
This error comes from parsing user-provided broker arguments/env values and propagates the raw {url:?} value through CLI output. The broker URL can include query strings or userinfo, so print only the configuration name and the required scheme.
Proposed fix
- bail!("{what} {url:?} must be https (or loopback http for local development)");
+ bail!("{what} must be https (or a permitted loopback HTTP endpoint for local development)");📝 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.
Suggested change
| bail!("{what} {url:?} must be https (or loopback http for local development)"); | |
| bail!("{what} must be https (or a permitted loopback HTTP endpoint for local development)"); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@cmux-tui/crates/cmux-tui-iroh/src/broker.rs` at line 87, Update the
validation error in the broker URL parsing flow to omit the raw URL value. Keep
the configuration name and required HTTPS-or-loopback-HTTP guidance in the
message, but remove the `{url:?}` interpolation so query strings and userinfo
cannot reach CLI output.
Source: Coding guidelines
Comment on lines
+134
to
+136
| result = json.loads(self.reader.readline(30.0, "transport handshake")) | ||
| if not result.get("accepted"): | ||
| raise RuntimeError(f"transport handshake rejected: {result}") |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Files:\n'
git ls-files | grep -E '(^|/)attach-once\.py$|(^|/)mint-enrollment-token\.mjs$|(^|/)entrypoint\.sh$|(^|/)cmux-tui-iroh/src/main\.rs$' || true
printf '\nattach-once outline:\n'
ast-grep outline cmux-tui/scripts/iroh-docker-demo/attach-once.py --view compact || true
printf '\nattach-once relevant lines:\n'
cat -n cmux-tui/scripts/iroh-docker-demo/attach-once.py | sed -n '1,90p;120,150p;166,286p'
printf '\nentrypoint main and attach-once callers:\n'
cat -n cmux-tui/scripts/iroh-docker-demo/entrypoint.sh | sed -n '1,220p'
printf '\nmint-enrollment relevant lines:\n'
cat -n cmux-tui/scripts/iroh-docker-demo/mint-enrollment-token.mjs | sed -n '1,110p'
printf '\nTop-level Python error handling/search:\n'
rg -n "RuntimeError|argparse|print Exception|traceback|except Exception|catch Exception|process\.stderr" cmux-tui/scripts/iroh-docker-demo -S || true
printf '\nPython behavioral probe: exception string includes raw payload fields:\n'
python3 - <<'PY'
import json
result = {"accepted": False, "provider_id": "prov-SECRET", "status": {"code": 500, "message": "bad token"}}
msg = f"transport handshake rejected: {result}"
print(msg)
print("secret_payload_in_exception=", "prov-SECRET" in msg)
PY
printf '\nPython argument parser behavior probe with RuntimeError (read-only mock path):\n'
python3 - <<'PY'
import io, sys
try:
with open(__import__('tempfile').NamedTemporaryFile(delete=False).name) as f:
raise TypeError("boom")
except Exception as exc:
print(exc)
PYRepository: manaflow-ai/cmux
Length of output: 18391
Information Disclosure (CWE-209): Generation of Error Message Containing Sensitive Information
Reachability path
● Entry
cmux-tui/crates/cmux-tui-iroh/src/endpoint.rs:65
spawn_relay_maintenance: Exponential backoff between rotation attempts keeps a persistently
│
▼
● Sink
cmux-tui/scripts/iroh-docker-demo/attach-once.py
Avoid leaking raw handshake response data in attach-once.py.
RuntimeError(f"transport handshake rejected: {result}") includes provider handshake fields in the Python error string. Use a stable product-level message for the exception and log only a bounded internal diagnostic code if needed.
cmux-tui/scripts/iroh-docker-demo/attach-once.py#L134-L136
🧰 Tools
🪛 Ruff (0.16.0)
[warning] 136-136: Avoid specifying long messages outside the exception class
(TRY003)
📍 Affects 2 files
cmux-tui/scripts/iroh-docker-demo/attach-once.py#L134-L136(this comment)cmux-tui/scripts/iroh-docker-demo/mint-enrollment-token.mjs#L60-L62
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@cmux-tui/scripts/iroh-docker-demo/attach-once.py` around lines 134 - 136,
Replace the raw response interpolation in the transport-handshake rejection
raised by the reader logic in attach-once.py with a stable product-level error
message; if diagnostics are needed, log only a bounded internal code rather than
provider fields. In cmux-tui/scripts/iroh-docker-demo/mint-enrollment-token.mjs
lines 60-62, apply the same redaction behavior to the corresponding handshake
rejection site.
Source: Coding guidelines
Collaborator
|
Closing this superseded stage-1 sidecar. Head |
Contributor
|
Closing as superseded by the current relay transport architecture. Its parent Linux enrollment work is closed, and the sidecar protocol does not apply cleanly to current main. |
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
3 participants
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.
Stage 1 of the iroh TUI transport program: a
cmux-tui-irohsidecar that makes any cmux-tui session reachable by dialing its EndpointID alone through the cmux account device registry and the managed relay fleet, including from a Docker container behind NAT with zero inbound ports.The design doc, docs/iroh-tui-transport-stage1.md, maps each binding constraint of docs/iroh-app-transport-architecture.md to its stage-1 implementation, including the relationship to the existing
cmux-remotedaemon (different trust system; this sidecar is the account-scoped, arch-conformant path) and the explicitly deferred items.listenregisters the device's (user, deviceId, tag) slot with the trust broker, binds a Minimal-preset endpoint on the verified relay catalog (never n0 defaults or public DNS discovery), and admits a connection only when its first stream carries a broker-signedcmux/tui/1pair grant naming this exact acceptor and the TLS-authenticated initiator. Admitted streams bridge byte-for-byte to the session Unix socket, so the protocol v10 JSON-lines contract crosses unchanged under the documented relay-stdio authority model. Broker state revalidates every 30 s, grant expiry closes idle sessions, and relay credentials rotate lazily (remove/insert_relay after ~30 s confirmed down, honoring mint quotas).enrollexchanges a one-use provisioning token for a Stack session credential (headless: no interactive sign-in in a container), mints the persistent Ed25519 endpoint key + deviceId into the state root with owner-only atomic file handling, and registers the slot; restarts re-register the same slot as a heartbeat.providerimplements machine-provider v1 over stdio forcmux-tui --machine-provider-command: the control process owns discovery, pair grants, and the single iroh endpoint, and per-ticket stream processes rendezvous over a private Unix socket so a second endpoint never binds the same key.Tests: 19 unit/integration tests including a real-iroh localhost round trip (admission with a signed grant, byte-faithful bridge to a session socket, denial of a wrong-acceptor grant) and a golden test pinning the embedded relay catalog to
config/iroh/managed-relay-catalog.json.scripts/iroh-docker-demo/carries the stage-1 acceptance harness (zero-port container, EndpointID-only attach, detach/reattach, restart re-registration); the recorded demo evidence lands in this PR's conversation.Broker counterpart (
linuxplatform, TUI pair-grant profile, enrollment routes): #9515. This PR is independent to build but needs 9515's routes deployed to exercise the live flow.Dictionary: relay-stdio (the existing cmux-tui transport class where a remote principal's bytes terminate at the local session Unix socket, receiving local-admin authority), pair grant (broker-signed JWS binding two same-account device endpoints to an ALPN and scope), managed relay fleet (the cmux-operated iroh relays in
config/iroh/managed-relay-catalog.json), machine-provider v1 (the versioned stdio contract by which a provider hands the TUI reader/writer halves for a remote session).🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by cubic
Adds a broker-registered
irohtransport sidecar (cmux-tui-iroh) that makes anycmux-tuisession reachable by EndpointID through the account registry and the managed relay fleet, including behind NAT with zero inbound ports. Requires the broker routes from PR 9515 for the full flow.New Features
cmux-tui-irohbinary withenroll,listen, and machine-providerprovider control|stream.irohendpoint on the verified relay catalog only, admits ALPNcmux/tui/1with a broker-signed pair grant, then bridges streams to the session Unix socket.cmux-tui --machine-provider-command, with per-ticket rendezvous over a private Unix socket.<state root>/device(iroh-identity.json,iroh-credential.json,iroh-broker-cache.json) with broker revalidation and relay-token rotation.irohround trip.Bug Fixes
--as a separator.Written for commit 4661b2a. Summary will update on new commits.
Summary by CodeRabbit