config: advertise the non_selectable_outbounds capability#555
Conversation
Report common.CapabilityNonSelectableOutbounds in the config request's Capabilities. This build honors ConfigResponse.NonSelectableOutbounds (merges those outbounds but excludes them from the proxy-selection groups), so the server can gate infrastructure outbounds (e.g. the proxyless rule-set download_detour) on the capability instead of the client version — an old client that would surface such an outbound as a selectable proxy simply never advertises it. Bumps getlantern/common to the branch adding Capabilities; will re-point to merged main once getlantern/common#28 lands. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WGpKfgkbfdJwwsTaouMDoR
|
Warning Review limit reached
Next review available in: 55 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe config fetch request now advertises a ChangesConfig request capability update
Estimated code review effort: 1 (Trivial) | ~3 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
There was a problem hiding this comment.
Pull request overview
This PR adds an explicit capability signal to the client’s config fetch request so the server can safely gate delivery of “infrastructure/non-selectable” outbounds based on client behavior rather than client version.
Changes:
- Adds
common.CapabilityNonSelectableOutboundsto the config request’sCapabilitiespayload (config/fetcher.go). - Bumps
github.com/getlantern/commonto a revision that includes the newConfigRequest.Capabilitiesfield + capability constant (go.mod,go.sum).
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| config/fetcher.go | Advertises non_selectable_outbounds capability in config fetch request. |
| go.mod | Updates github.com/getlantern/common dependency to include the new request field/constant. |
| go.sum | Updates sums corresponding to the common dependency bump. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@coderabbitai full review |
✅ Action performedFull review finished. Your plan includes PR reviews subject to rate limits. More reviews will be available in 50 minutes. |
common#28 (Capabilities field) landed; replaces the pre-merge branch pseudo-version with the merge commit on main. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
config/fetcher.go (1)
88-91: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider asserting
CapabilitiesinTestFetchConfig.The existing test unmarshals the request into
C.ConfigRequestand asserts several fields but doesn't verifyCapabilitiescontainsC.CapabilityNonSelectableOutbounds, per the test snippet inconfig/fetcher_test.go. Adding this assertion would guard against future regressions in the request payload.🤖 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 `@config/fetcher.go` around lines 88 - 91, Add an assertion in TestFetchConfig to verify the request payload includes Capabilities with C.CapabilityNonSelectableOutbounds. Locate the request unmarshalling and existing field checks in fetcher-related test code, and extend the assertions so the capability is explicitly validated alongside the other C.ConfigRequest fields.
🤖 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.
Nitpick comments:
In `@config/fetcher.go`:
- Around line 88-91: Add an assertion in TestFetchConfig to verify the request
payload includes Capabilities with C.CapabilityNonSelectableOutbounds. Locate
the request unmarshalling and existing field checks in fetcher-related test
code, and extend the assertions so the capability is explicitly validated
alongside the other C.ConfigRequest fields.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: c461191b-1a0f-4cd1-8bfb-1f1eebacb7b9
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (2)
config/fetcher.gogo.mod
Server-side infra-outbound gating depends on the client advertising non_selectable_outbounds; a refactor dropping it should fail tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
What
Advertise
common.CapabilityNonSelectableOutbounds("non_selectable_outbounds") in the config request'sCapabilities(config/fetcher.go).Why
radiance#549 made this client honor
ConfigResponse.NonSelectableOutbounds— it merges server-declared infrastructure outbounds into the box config so their references resolve, but excludes them from both proxy-selection groups (auto/URLTest and the manual selector). Advertising the capability lets lantern-cloud gate infrastructure outbounds on it instead of the client version.The concrete case is the proxyless rule-set download_detour (lantern-cloud#2936): a client that doesn't honor
NonSelectableOutboundswould let its URLTest auto-select the low-latency direct+frag dialer and route traffic through it, exposing the real IP. Because this capability is added in the same client that has #549's handling, "advertises it" ⟺ "handles it safely" — a true capability signal, and one gate covers every future infra outbound with no new client release.(The client already advertises the outline outbound type
"outline"via its auto-derived supported-protocols list, but that predates #549, so it can't be the gate — old clients report it too. This is a distinct, explicitly-added capability.)Dependencies
ConfigRequest.Capabilities+ the constant. Merged —go.modnow points at the merge commit oncommonmain (5d90dcb).Testing
go build ./config/..., gofmt clean;go test ./config/passes against mergedcommonmain. (The advertised value round-trips throughcommon'sTestCapabilitiesRoundTrip.)🤖 Generated with Claude Code
https://claude.ai/code/session_01WGpKfgkbfdJwwsTaouMDoR
Summary by CodeRabbit