fix(hls,auth): redact credentials from URLs in errors

[kixelated]

reqwest's Display and Debug include the full dialed URL, so an HLS playlist or segment URL, or a moq-auth server URL, carrying ?jwt= leaked its token into logs. This applies the same without_url() fix #4457 made in moq-rtc.

• moq-hls: From<reqwest::Error> strips the URL before wrapping.
• moq-auth: reqwest::Error leaves the from_message! macro for a hand-written impl that strips the URL before flattening.
• moq-auth: InsecureUrl and InvalidUrl echoed the configured URL verbatim. They now drop userinfo, query, and fragment.

Each crate gets an http_error_redacts_url regression test that dials a closed port with ?jwt=secret and checks that neither Display nor Debug contains the secret. url_schemes_are_checked_at_construction now also checks that a refused URL is redacted. All three fail without the fix.

I audited the other reqwest call sites and left them unchanged:

• moq-tokio fingerprint bootstrap already clears the query, and reqwest never prints userinfo.
• moq-relay cluster.connect_api logs anyhow's %err, which prints only the outermost context, not the reqwest source. without_url() can't redact the chain there anyway: http-cache wraps the reqwest error in its own anyhow error, so a fix would need a custom middleware.
• moq-cli auth reports the operator's own --internal-url back to them.

Public API / wire impact: none. Error messages lose the URL; variants are unchanged.

(Written by Claude Opus 5.5)

🤖 Generated with Claude Code

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: fc095701-333a-4f5b-933a-11375ff71a99

📥 Commits

Reviewing files that changed from the base of the PR and between 7f5ae43 and 1f64b67.

📒 Files selected for processing (3)

• rs/moq-auth/src/client.rs
• rs/moq-auth/src/error.rs
• rs/moq-hls/src/error.rs

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 0 remain after this review.

---

Walkthrough

The changes redact userinfo, query parameters, and fragments from client-generated URL errors in moq-auth. The moq-auth and moq-hls conversions from reqwest errors remove the URL before exposing or storing those errors. Tests check that error display and debug output do not contain credentials or query secrets.

Priority: ➖ Normal

Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to 1f64b

No concrete merge-blocking risk remains: the changed error paths redact the documented credential locations, and the path concern was not supported by the repository contract.

Security Architecture Review

Security architecture risk: ⚪ Minimal · up to 1f64b

The reviewed changes reduce credential exposure in errors without widening accepted URLs, changing transport security, or changing error categories. No material security risk introduced or worsened by this PR was identified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The security benefit applies to consumers formatting or retaining errors produced through these auth and HLS conversion paths. It does not establish that every repository logging or request-error path is sanitized.

Trust Boundaries and Controls

• observed — The auth constructor still rejects non-loopback HTTP and unsupported schemes, retains HTTPS TLS configuration handling, and uses the Unix socket as transport with a localhost request target. The changed rejection branches sanitize errors rather than weaken admission controls.

Resilience and Maintainability Implications

• observed — Regression tests assert that Display and Debug omit supplied URL secrets after connection failure and that rejected auth URLs omit userinfo, query, and fragment. These assertions support the intended diagnostic control; execution results were not available in this assessment.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the main change: redacting credentials from URLs in HLS and authentication errors.
Description check	✅ Passed	The description directly explains the URL-redaction changes, affected crates, regression tests, scope, and lack of public API impact.
Linked Issues check	✅ Passed	Issue [#4457] is closed. It supplies historical context only. No active directly linked issue remains, so this pull request has no linked-issue coding requirements.
Out of Scope Changes check	✅ Passed	The reported changes stay within the pull request scope. moq-hls and moq-auth remove URLs from reqwest::Error output, and moq-auth also redacts userinfo, query, and fragment data in URL valida…
Docstring Coverage	✅ Passed	Docstring coverage is 81.82% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 11 functions across 3 files.

✨ Finishing Touches

✨ Simplify code

• Commit to this branch
• Create a new PR

---

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.}

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-09-29T20:26:59.473542Z	4eaf6a7	New commits

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[kixelated]

MERGE

Positive security improvement and a direct follow-up to the #4457 audit note that moq-hls / moq-auth still wrapped reqwest::Error without stripping the dialed URL. JWT/query tokens and userinfo on playlist, segment, or auth-server URLs would otherwise land in Display/Debug logs — same class of leak #4457 fixed in moq-rtc.

Complexity is minimal and well placed: both From<reqwest::Error> paths call without_url() at the conversion boundary (moq-auth correctly leaves the from_message! macro for a hand-written impl). No public API or wire change; only error text loses the URL, which callers already know.

Regression tests mirror the #4457 closed-port pattern and assert both Display and Debug omit jwt / secret / user:pass. The author's audit of remaining call sites (moq-tokio fingerprint bootstrap, moq-relay cluster.connect_api, moq-cli auth) is sound enough not to block — no alternate approach (e.g. in-place URL mutation) is clearly better than the established without_url() pattern.

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

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

[kixelated]

Automated review by review (OpenAI)

Reviewed commit d41e426.

Direction: good, narrowly scoped fix. Redacting at the reqwest-to-domain-error boundary avoids relying on every logging call to remember to strip credentials. Keeping the error variants unchanged is appropriate.

No actionable regression found in the refreshed diff. Tests exercise the error boundary; this review did not independently rerun those network tests.

Verification: static diff and targeted source inspection. I did not run the repository test/build suites or reproduce runtime scenarios in this review. This is a COMMENT review, not a merge approval.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

[kixelated]

MERGE (follow-up)

Reviewed head 1f64b6790da9d3b1ae3d80849c3bf8c50a4e8f19. Prior Grok review covered the without_url() conversion only; this push adds the second commit that redacts credentials from refused auth URLs.

What changed since the last Grok review

• moq-auth: redact() strips userinfo / query / fragment before InsecureUrl and InvalidUrl.
• Construction test now asserts a refused URL keeps the host and drops jwt / userinfo / fragment.

That closes the gap the first review did not cover: a misconfigured http://…?jwt= or ftp://user:pass@… auth URL was echoed verbatim into the startup error.

Residual (non-blocking)
moq-hls still embeds a full url::Url in MissingByteRangeOffset, InvalidByteRange, ByteRangeLengthMismatch, and ByteRangeResponseMismatch (rs/moq-hls/src/error.rs + import.rs). Those Display forms will still print ?jwt= on a signed segment URL if a byte-range check fails. Out of this PR’s stated From<reqwest::Error> / construction-error scope, but the same leak class — worth a follow-up that redacts (or stores a redacted string) before formatting, same as redact() here.

No other concrete issues in the new delta. without_url() at the reqwest boundary matches #4457; tests cover both Display and Debug. CI was still queued at review time.

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

[kixelated]

Merge summary

Rebased onto origin/main to clear the macOS awk: newline in string failure, which #4568 fixed on main. One conflict in rs/moq-auth/src/client.rs: main had refactored an ask helper into the exact spot where the branch added redact. Resolved by keeping both, so redact now sits just after ask. Net diff unchanged: 3 files, +70/-6.

Checks on the rebased head

• just check passes (676 tests run, 676 passed, 2 skipped).
• macOS, Windows, Check, and Test all green.

Regression tests spot check. I reverted both without_url() calls and re-ran http_error_redacts_url. Both fail without the fix, and the panic output shows the leak itself, so the tests genuinely bind:

moq-hls  error leaked jwt: reqwest: error sending request for url (http://127.0.0.1:34233/media.m3u8?jwt=secret) Reqwest(reqwest::Error { kind: Request, url: "...jwt=secret", ... })
moq-auth error leaked jwt: auth server unavailable: error sending request for url (http://127.0.0.1:37227/?jwt=secret): client error (Connect): tcp connect error: ...

Worth noting the moq-auth leak arrives through message()'s source() walk, not only through reqwest's own Display, so without_url() has to happen before flattening. Both restored afterward; the tree matches the merged head.

On the byte-range residual. Agreed that MissingByteRangeOffset, InvalidByteRange, ByteRangeLengthMismatch, and ByteRangeResponseMismatch are the same leak class. Leaving them out of this PR, because the fix is a public API shape decision on a published crate (moq-hls 0.5.8), and there are two defensible answers:

1. Change url: url::Url to a redacted String. Clean, but a semver break, so it belongs on dev.
2. Keep url::Url and store a sanitized clone at construction. Additive, but the error then names a URL that was not the one dialed, which is its own kind of confusing.

That is the maintainer's call rather than something to slip into a security fix targeting main. Suggest a follow-up quest for it.

No public API or wire change here. No version bumps.

(Written by Space Bunny Free)