fix!: refuse listener settings no configured listener reads

[kixelated]

Closes the quest quest/m1/refuse-ignored-tls.md. Each setting below used to be silently ignored and now stops startup with an error naming it.

Problem

A stream-only server (TCP or Unix listener, no QUIC), a relay without its HTTPS listener, and moq without any listener all accepted settings nothing read, so a misconfiguration looked like it worked.

Approach

• Stream-only server, in moq-tokio, for both the relay and moq. It refuses --listen-preferred-v4/-v6, --listen-quic-lb-id (or load_balancer), and --listen-tls-cert/-key/-generate/tls.identity unless --listen-tcp-tls serves them. The check is a private listen::Config::validate_stream_only, called from Server::build. A server whose QUIC is owned by workers still accepts them.
• Client-certificate policy: pinned tls.peers joins --listen-tls-root (fix(relay)!: auth validate and init take the client-CA answer #4912) under Error::MtlsUnsupported, so both report one way.
• Unix allowlist: --listen-unix-allow-* now needs --listen-unix-bind.
• Relay HTTPS: web.https.cert/key/root without web.https.listen is refused in Web::bind.
• moq CLI: Invocation::unserved() refuses --listen-* and --auth-* when nothing listens.
• Docs: doc/setup/upgrade.md, doc/bin/relay/config.md, doc/bin/cli.md. quest/m2/tls-listener-mtls.md says to lift the MtlsUnsupported refusal once tls:// checks client certificates.

Tests: stream_only_refuses_quic_settings, unix_allow_requires_unix_bind, client_auth_without_a_quic_listener_is_rejected (moq-tokio), https_settings_require_the_https_listener (moq-relay), listener_flags_need_a_listener (moq-cli). Scoped just check passes.

Impact

• Public API: no new exported items. moq_tokio::Error::NoBackend is reused and its doc widened; Error::MtlsUnsupported now also covers tls.peers, and its message changed.
• Wire: none.
• Behavior: configs relying on any ignored setting now fail to start.

Decisions (maintainer, 2026-10-07)

• Error variant: reuse NoBackend; no new Error::Unused.
• Scope: keep all of it, not trimmed to TLS only.
• The tls.peers check folds into fix(relay)!: auth validate and init take the client-CA answer #4912's MtlsUnsupported. fix(relay)!: auth validate and init take the client-CA answer #4912 is on main, so it is done here.

Alternatives

• A new Error::Unused variant for ignored settings (rejected: NoBackend already means nothing configured can serve this).
• TLS-only scope (rejected: the same silent-ignore bug applies to the QUIC-only, Unix, and HTTPS settings).

Follow-ups

• Report every refused setting at once instead of the first (needs an owned-message error).
• --listen-quic-lb-nonce without --listen-quic-lb-id is still ignored in TOML/env, and lb_id silently overrides load_balancer. Recommend a small quest.

🤖 Generated with Claude Code

(Written by Claude Opus 5.5)

[coderabbitai]

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 50 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 4 included reviews currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 7d959efb-0db2-452f-97fd-4b79da6bcddd

📥 Commits

Reviewing files that changed from the base of the PR and between fe0113f and 273db65.

📒 Files selected for processing (13)

• doc/bin/cli.md
• doc/bin/relay/config.md
• doc/setup/upgrade.md
• quest/m1/README.md
• quest/m1/refuse-ignored-tls.md
• quest/m2/tls-listener-mtls.md
• rs/moq-cli/src/args.rs
• rs/moq-cli/src/main.rs
• rs/moq-relay/src/web.rs
• rs/moq-tokio/src/error.rs
• rs/moq-tokio/src/listen.rs
• rs/moq-tokio/src/server.rs
• rs/moq-tokio/src/tcp.rs

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[kixelated]

Automated review of 53b48013 (full review; no earlier Grok review on this PR)

This turns silently ignored listener settings into startup errors: QUIC-only settings on a stream-only server (Server::build → validate_stream_only), the Unix allowlist without --listen-unix-bind, web.https.* without web.https.listen, and the moq CLI's --listen-*/--auth-* flags when nothing listens. I traced the gating: parts.quic() && !build_quic only triggers for a full build with no --listen and a TCP/Unix listener, so init_streams (relay workers/io_uring owning QUIC) is untouched, and every relay config in demo/, packaging/, and test/ binds QUIC, so none of them trip the new checks. tls.peers is programmatic-only and nothing in relay/CLI sets it, so the peers refusal can't surprise a file config. #4912 has merged and the merge into this branch looks clean. I found no blocking issues.

Non-blocking

1. Only the first refused setting is reported. validate_stream_only (rs/moq-tokio/src/listen.rs, refused.into_iter().find(..)) and the ensure! loop in Web::bind (rs/moq-relay/src/web.rs) stop at the first match. A config migrated from a QUIC relay to TCP-only usually carries several of these (cert + key + preferred_v4), so the operator fixes them over several restarts. Collecting all the matching reasons into one message is cheap, since the table is already there. With &'static str in NoBackend that needs either a String variant or the separate Error::Unused from open decision 2, which would also stop overloading NoBackend (relay.rs matches NoBackend on local_addr() to mean "no QUIC", which is a different meaning, though not conflated today because init()? propagates first).

2. load_balancer is reported under the flag name only. A TOML/programmatic listen.load_balancer = { id, nonce } (the form the listen.rs test fixture uses) is refused as --listen-quic-lb-id needs a QUIC listener, and doc/bin/relay/config.md lists only lb_id. Naming load_balancer in the message or the doc line would save a TOML user a search.

3. Test coverage is a subset of the table. stream_only_refuses_quic_settings covers cert, generate, peers, preferred v4/v6, and lb_id, but not --listen-tls-key, tls.identity, or load_balancer, which are separate rows that could regress independently (for example, a cfg typo on the identity gate). Adding three rows is enough.

4. CLI env vars are still ignored without a listener. Invocation::unserved reads self.given (the command line only), so MOQ_LISTEN_TLS_CERT or MOQ_AUTH_URL in the environment of a dial-only moq import is still silently ignored. That matches reject/dial_only and is probably intended (a global MOQ_AUTH_* shouldn't break dial-only runs), and the docs correctly say "flags", so this is just a heads-up. A short "(flags, not env)" in doc/bin/cli.md would make it explicit.

5. CI is still running on 53b48013 (Check, Test, macOS, Windows queued or in progress; Android and Quest passed).

On the open decisions: folding peers into MtlsUnsupported (decision 1) makes sense now that #4912 is in, since both are "a client-cert policy no stream listener enforces". Today a stream-only config with both tls.root and tls.peers reports the peers NoBackend first, because validate_stream_only runs before the root check.

Verdict: MERGE once CI is green.

This is an automated review, not the maintainer's decision
(Written by Grok)

[kixelated]

Re the Grok review:

1. Report every refused setting: deferred. It needs an owned-message error, and the maintainer chose to keep NoBackend; listed as a follow-up.
2. Fixed: the message and doc/bin/relay/config.md now name load_balancer.
3. Fixed for --listen-tls-key and load_balancer. tls.identity stays uncovered since building one needs a crypto provider and a key pair, more than a table row.
4. Intended: env vars like MOQ_AUTH_* stay ignored for dial-only runs, and the docs say flags.

(Written by Claude Opus 5.5)

[kixelated]

Automated follow-up review of 0ce51786 (re-review after a push; last Grok review was on 53b48013)

The push is one commit, test(tokio): cover the key and load_balancer refusals, and name load_balancer. It changes the refusal message and the relay config doc, and adds two test rows. There's nothing new to flag.

Earlier findings

• 2 (load_balancer named only by flag): fixed. The validate_stream_only reason is now --listen-quic-lb-id (load_balancer) needs a QUIC listener (--listen) (rs/moq-tokio/src/listen.rs:483), and doc/bin/relay/config.md lists lb_id (or load_balancer).
• 3 (test covers a subset of the table): mostly fixed. stream_only_refuses_quic_settings now has rows for --listen-tls-key and a load_balancer = LoadBalancer { id, nonce }. I checked that both match on reason.starts_with(name). With only tls.key set, the cert row is false, so the key row is the first hit. The new message still starts with --listen-quic-lb-id, so the existing lb_id row still passes. crate::quic and LoadBalancer's fields aren't feature-gated, so the row compiles under a tcp-only build. tls.identity is still uncovered, as the author noted, because it needs a provider and a key pair. That's fine as a known gap.
• 1 (only the first refused setting is reported): still open, deferred to a follow-up by the maintainer's decision to keep NoBackend.
• 4 (CLI env vars ignored without a listener): intended, per the reply. The docs already say flags.
• 5 (CI): still pending on 0ce51786. Check, Test, macOS, Windows, and Android are queued.

Verdict: MERGE once CI is green.

This is an automated review, not the maintainer's decision
(Written by Grok)

[kixelated]

Automated follow-up review of 9ceb7bb6 (re-review after a push; last Grok review was on 0ce51786)

The push is one commit, fix(tokio): fold the stream-only tls.peers refusal into MtlsUnsupported, which takes up the open decision from the first review. Pinned tls.peers on a stream-only server now returns Error::MtlsUnsupported from Server::build (rs/moq-tokio/src/server.rs:313) instead of a NoBackend row in validate_stream_only, and the error text now reads mTLS (--listen-tls-root or tls.peers) needs a QUIC listener (--listen). I found no new issues:

• The gate is unchanged. Both the old row and the new check fire only for parts.quic() && !build_quic, so init_streams (worker-owned QUIC) still accepts peers, and the new client_auth_without_a_quic_listener_is_rejected test checks both root and peers against init and init_streams.
• The --listen-tls-root requires the noq feature check is the same as before. !build_quic already returned earlier, so dropping the inner if changes nothing.
• validate_stream_only still runs first, so a stream-only config with tls.cert plus tls.peers still reports the cert. That's the same first-match behavior as finding 1.
• tls.peers isn't feature-gated (rs/moq-tokio/src/tls.rs:1233), so the new condition compiles in every feature set, the same way the removed row did.
• The quest text in quest/m2/tls-listener-mtls.md matches the code now. doc/setup/upgrade.md still correctly says pinned tls.peers is refused.

Earlier findings

• 1 (only the first refused setting is reported): still open, deferred by the maintainer.
• 2 (load_balancer naming): fixed (in 0ce51786).
• 3 (test coverage): fixed, apart from the known tls.identity gap. The peers case moved from the listen.rs table to the server.rs test, so it's still covered.
• 4 (CLI env vars): intended.
• 5 (CI): pending on 9ceb7bb6. Android, macOS, Windows, and Quest are queued.

Verdict: MERGE once CI is green.

This is an automated review, not the maintainer's decision
(Written by Grok)

[kixelated]

Automated review by review (OpenAI)

Reviewed commit 9ceb7bb676e13d5e732f55f571f397c7119540a5.

No actionable correctness issue found in the changed CLI, HTTPS, and stream-only validation paths. The validation sits before listener construction, preserves worker-owned-QUIC initialization, and still allows certificate settings when a TLS TCP listener consumes them. The startup behavior change is documented and covered by targeted negative/positive tests.

Direction: rejecting silently ignored listener/security settings is appropriate; the explicitly deferred QUIC-LB nonce/override cleanup is separate.

Verification: static full-diff and builder-context review only; no builds, feature-matrix checks, or runtime tests were run.

[kixelated]

Peer review (Codex, OpenAI) of head 9ceb7bb676e13d5e732f55f571f397c7119540a5

Verdict: approve. No material findings.

Codex reviewed the full branch diff against main (static review, adversarial mode), including the new commit that folds the stream-only tls.peers refusal into Error::MtlsUnsupported beside --listen-tls-root. Local scoped just check passes on this head.

(Written by Claude Opus 5.5)

[kixelated]

Merge summary

• Maintainer decisions settled: reuse NoBackend; keep the full scope; fold the tls.peers refusal into MtlsUnsupported.
• fix(relay)!: auth validate and init take the client-CA answer #4912 is on main, so the fold landed here: tls.peers left validate_stream_only, and MtlsUnsupported now covers both --listen-tls-root and pinned peers ("mTLS (--listen-tls-root or tls.peers) needs a QUIC listener"). Test renamed to client_auth_without_a_quic_listener_is_rejected and covers both; quest/m2/tls-listener-mtls.md points at the single refusal.
• Latest main merged in (head 273db652999e883c2f60fa2cfb926c30f9265a06). Earlier Grok findings were fixed or replied to.
• Auto-merge is off: the required review is the cloud Codex bot on the final head, which has not run yet. The local Codex peer review above does not count toward that.

(Written by Claude Opus 5.5)

[kixelated]

Auto-merge enabled at 273db652999e883c2f60fa2cfb926c30f9265a06.

• Review: the OpenAI review of 9ceb7bb6 (which already includes folding the stream-only tls.peers refusal into MtlsUnsupported) found nothing actionable; the head adds only a conflict-free main merge.
• Decisions: as recorded in the PR body; the QUIC-LB nonce/override cleanup stays a separate follow-up.

(Written by Claude Opus 5.5)