[thomhurst]

SignalR distributed mode now rejects non-loopback HTTP listener and advertised URLs, with or without an access token. Workers apply the same rule to configured and discovered endpoints before logging the URL or attempting a connection, keeping bearer tokens and module payloads off plaintext remote connections.

HTTP remains available on loopback for local development and the local side of HTTPS proxies and cloudflared tunnels. The configuration guide includes HTTPS listener examples, certificate setup, and the breaking migration requirement.

Validation:

• SignalR solution Release build: passed, no warnings or errors.
• SignalR TUnit executable: all 155 tests passed, including endpoint validation, discovery rejection, token-free configurations, error redaction, and existing loopback connections.
• Changed C# files: whitespace formatting applied; scoped formatting verification passed at info severity.
• Documentation: yarn install --frozen-lockfile and yarn build passed on Node 24.22.0.

Fixes #7001.

Summary by CodeRabbit

• Bug Fixes
  • SignalR endpoints now accept HTTPS URLs and HTTP URLs targeting loopback. Non-loopback HTTP URLs are rejected during configuration validation or before a worker connects, including discovered endpoints.
• Documentation
  • Updated endpoint guidance and examples to reflect HTTPS requirements for non-loopback listeners, certificate trust, and using an HTTP listener behind an HTTPS-terminating proxy or tunnel.

[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-10-24T15:28:49.320674Z	ec9583d	PR opened

ℹ️ 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.

[coderabbitai]

No actionable comments were generated in the recent review.

Recent review info

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: af92f459-73ff-440b-ace2-51729bcd698e

Commits

Reviewing files that changed from the base of the PR and between e37ce5c and ec9583d.

Files selected for processing: 6

• docs/docs/distributed/configuration.md
• src/ModularPipelines.Distributed.SignalR/Coordination/SignalRDistributedCoordinatorFactory.cs
• src/ModularPipelines.Distributed.SignalR/SignalRDistributedOptions.cs
• src/ModularPipelines.Distributed.SignalR/SignalRDistributedOptionsValidator.cs
• test/ModularPipelines.Distributed.SignalR.UnitTests/ConfigurationTests.cs
• test/ModularPipelines.Distributed.SignalR.UnitTests/SignalREndpointOptionsTests.cs

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

Walkthrough

SignalR endpoint validation now allows absolute HTTPS URLs and loopback HTTP URLs. Worker creation also rejects unsafe configured or discovered endpoints before connecting. The tests and documentation reflect these rules.

Changes: SignalR Endpoint Security

Layer / File(s)	Summary
Configured endpoint validation src/ModularPipelines.Distributed.SignalR/SignalRDistributedOptions.cs, src/ModularPipelines.Distributed.SignalR/SignalRDistributedOptionsValidator.cs, test/ModularPipelines.Distributed.SignalR.UnitTests/SignalREndpointOptionsTests.cs, test/ModularPipelines.Distributed.SignalR.UnitTests/ConfigurationTests.cs, docs/docs/distributed/configuration.md	ListenUrl and configured AdvertisedUrl accept absolute HTTPS URLs or loopback HTTP URLs. Tests and documentation describe the accepted forms and validation failures.
Worker endpoint check src/ModularPipelines.Distributed.SignalR/Coordination/SignalRDistributedCoordinatorFactory.cs, test/ModularPipelines.Distributed.SignalR.UnitTests/SignalREndpointOptionsTests.cs, docs/docs/distributed/configuration.md	CreateWorkerAsync rejects unsafe configured or discovered endpoints before connection setup. Tests cover these failures, and the documentation describes certificate trust and HTTPS-terminating proxies or tunnels.

Priority: High

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix · Severity of issue fixed: High

Possibly related issues

• v4.0.0 release checklist #3997: The release checklist identifies SignalR transport security as remaining work, and this change enforces HTTPS for non-loopback endpoints.

Merge Risk: Minimal · up to ec958

No actionable merge-blocking issue remains; the HTTPS endpoint change is ready to merge after normal checks.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 12 functions across 5 files. (1 skipped: 1…	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly and concisely describes the main change: requiring HTTPS for non-loopback SignalR endpoints while allowing loopback HTTP.
Linked Issues check		Issue #7001 requires protection for non-loopback SignalR endpoints and documentation for the breaking behavior. The validator accepts HTTPS and loopback HTTP only for ListenUrl and AdvertisedUrl. …
Out of Scope Changes check		The whole pull request changes only SignalR endpoint validation, worker endpoint checks, related tests, and the distributed SignalR configuration documentation. These changes directly implement issue …

Full details: Linked Issues check

Explanation

Issue #7001 requires protection for non-loopback SignalR endpoints and documentation for the breaking behavior. The validator accepts HTTPS and loopback HTTP only for ListenUrl and AdvertisedUrl. CreateWorkerAsync applies the same check to configured and discovered endpoints before token resolution, URL logging, or connection. Tests cover non-loopback HTTP with and without tokens, unsafe discovered endpoints, redaction, and loopback HTTP. The documentation describes HTTPS setup, certificate trust, proxy and tunnel use, and migration impact.

Full details: Out of Scope Changes check

Explanation

The whole pull request changes only SignalR endpoint validation, worker endpoint checks, related tests, and the distributed SignalR configuration documentation. These changes directly implement issue #7001. No unrelated product behavior or files are demonstrated.

Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 12 functions across 5 files. (1 skipped: 1 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Finishing Touches: 💡 2

📝 Generate docstrings: 💡

• Commit to this branch
• Create a new PR

🛠️ Fix failing CI checks: 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autofix · 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

_{A rabbit checks each URL with care,
HTTPS takes the open air.
Loopback HTTP may stay nearby,
Unsafe links are stopped before they fly.
The worker waits for trust to grow,
Then through secure connections, off they go.}

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

[github-actions]

Claude Review

The PR makes SignalR distributed mode accept only HTTPS endpoints, or HTTP on loopback. A new IsSupportedEndpoint helper does the check. It is used in the options validator for ListenUrl and AdvertisedUrl, and in CreateWorkerAsync for the configured and discovered master endpoint. CreateWorkerAsync checks before the hub URL is built, anything is logged or a connection is attempted, and the exception message does not include the URL. The docs now cover HTTPS listener setup, certificate configuration and the breaking change. The tests cover loopback and HTTPS acceptance, non-loopback HTTP rejection with and without a token, invalid schemes, and worker rejection with no leaked secrets. I read the diff and the post-change factory code. I did not run the build or tests. I found no actionable issues.

Review evidence

src/ModularPipelines.Distributed.SignalR/SignalRDistributedOptionsValidator.cs: The new IsSupportedEndpoint helper requires an absolute URI that is either https or http on loopback. Uri.IsLoopback is false for 0.0.0.0, [::] and non-local hosts, so those are rejected. Both ListenUrl and AdvertisedUrl use it, and the failure messages mention HTTPS.

src/ModularPipelines.Distributed.SignalR/Coordination/SignalRDistributedCoordinatorFactory.cs: CreateWorkerAsync validates the resolved endpoint, whether it came from discovery or from options. It does so before GetHubUrl, logging, or connecting. The thrown message omits the URL and token, so credentials embedded in the URL cannot leak. This also covers endpoints that discovery returns, which options validation never sees.

test/ModularPipelines.Distributed.SignalR.UnitTests/SignalREndpointOptionsTests.cs: The tests cover accepted loopback variants (localhost, 127.0.0.2, ::1) and HTTPS. They cover rejection of non-loopback HTTP on both properties, with and without a token. They cover bad schemes and relative or null discovered endpoints on the worker. They also assert that the exception text does not contain the secrets embedded in the URL.

docs/docs/distributed/configuration.md: The docs update the examples and property table to HTTPS. They explain Kestrel certificate setup and the reverse-proxy and cloudflared pattern, where the local listener stays on loopback HTTP. They state the breaking change for non-loopback HTTP, which matches the implemented behavior.

test/ModularPipelines.Distributed.SignalR.UnitTests/ConfigurationTests.cs: Existing tests that used http://0.0.0.0 now use https://0.0.0.0. This keeps them valid under the new rule, and their assertions on token generation and failure are unchanged.

No actionable findings.

Optional follow-up notes

Master-side validation applies to ListenUrl, but a master on an HTTPS ListenUrl with no Kestrel certificate configured will fail only at server startup. The new docs mention the certificate setup, so this is acceptable. A friendlier startup error could be a follow-up.

endpoint.Url is passed to IsSupportedEndpoint before any null check on the MasterEndpoint. The helper accepts a nullable Uri, and the tests cover a null URL, so this is handled.

[greptile-apps]

Confidence Score: 5/5

[Medium impact] The PR appears safe to merge.

Summary

SignalR now requires HTTPS for non-loopback listener, advertised, and discovered URLs, with or without an access token.

• Remote SignalR endpoints use HTTPS, while loopback HTTP remains allowed.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Configured listener and advertised URLs] --> B[Check options]
    B --> C{HTTPS or loopback HTTP?}
    C -->|No| D[Reject configuration]
    C -->|Yes| E[Allow startup]
    F[Configured or discovered worker URL] --> G[Check worker endpoint]
    G --> H{HTTPS or loopback HTTP?}
    H -->|No| I[Throw without including URL]
    H -->|Yes| J[Build hub URL and connect]

Loading

_{Reviews (1) · Last reviewed commit: "Require HTTPS for non-loopback SignalR e..." · Reviewed by Greptile}