Skip to content

Wire BanditURLOverrides through to MutableURLTest#333

Draft
myleshorton wants to merge 8 commits intomainfrom
bandit-url-overrides
Draft

Wire BanditURLOverrides through to MutableURLTest#333
myleshorton wants to merge 8 commits intomainfrom
bandit-url-overrides

Conversation

@myleshorton
Copy link
Contributor

Summary

  • Passes BanditURLOverrides from config response through to sing-box MutableURLTestOutboundOptions.URLOverrides
  • Updates urlTestOutbound() and appendGroupOutbounds() to accept and forward urlOverrides map[string]string
  • Lantern server group gets bandit overrides; user group and autoAll pass nil
  • Updates go.mod to reference common and lantern-box bandit-url-overrides branches

Test plan

  • go vet ./vpn/... passes
  • Existing TestConnection and TestUpdateServers still pass
  • Verify MutableURLTest uses per-proxy callback URLs when overrides are present

🤖 Generated with Claude Code

Pass per-proxy callback URLs from config response through to
sing-box MutableURLTest URLOverrides, enabling per-ISP bandit
proxy assignment on the client side.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings February 20, 2026 17:11
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR threads BanditURLOverrides from the loaded config through to Lantern’s MutableURLTestOutboundOptions.URLOverrides, enabling per-proxy URL test callback overrides to reach the sing-box mutable URL test implementation.

Changes:

  • Extend urlTestOutbound() and appendGroupOutbounds() to accept urlOverrides map[string]string and forward it into MutableURLTestOutboundOptions.URLOverrides.
  • Pass cfg.BanditURLOverrides into the Lantern server group’s URL test outbound; keep User/All URL test outbounds using nil.
  • Update module dependencies (go.mod/go.sum) to versions/branches that include the new options surface.

Reviewed changes

Copilot reviewed 4 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
vpn/vpn.go Updates pre-start URL test outbound construction to the new urlTestOutbound signature (currently passing nil overrides).
vpn/tunnel_test.go Updates tests to call urlTestOutbound with the new signature.
vpn/boxoptions.go Wires BanditURLOverrides into the Lantern group’s MutableURLTest outbound and adds URLOverrides to the outbound options.
go.mod Bumps common/lantern-box (and other deps) to versions needed for the new override option surface.
go.sum Corresponding checksum updates after dependency changes.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

vpn/vpn.go Outdated
tags = append(tags, ob.Tag)
}
outbounds = append(outbounds, urlTestOutbound("preTest", tags))
outbounds = append(outbounds, urlTestOutbound("preTest", tags, nil))
Copy link

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

preTest() loads the config into cfg, but still calls urlTestOutbound("preTest", tags, nil), so BanditURLOverrides from the config are ignored during pre-start URL tests. If overrides are meant to affect URL testing behavior, they should be forwarded here as well (e.g., pass cfg.BanditURLOverrides).

Copilot uses AI. Check for mistakes.
slog.Debug("Merged config options", "tags", lanternTags)

appendGroupOutbounds(&opts, servers.SGLantern, autoLanternTag, lanternTags)
appendGroupOutbounds(&opts, servers.SGLantern, autoLanternTag, lanternTags, cfg.BanditURLOverrides)
Copy link

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR introduces URLOverrides wiring for MutableURLTest, but the existing buildOptions tests only validate Outbounds membership. Consider adding an assertion that the autoLanternTag MutableURLTest outbound includes the expected URLOverrides when cfg.BanditURLOverrides is set, to prevent regressions.

Copilot uses AI. Check for mistakes.
github.com/sagernet/sing v0.7.18
github.com/sagernet/sing-box v1.12.13
github.com/sagernet/sing-dns v0.4.6
github.com/sagernet/sing-box v1.12.22
Copy link

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

github.com/sagernet/sing-box is still replaced with github.com/getlantern/sing-box-minimal v1.12.19-lantern (see replace directive at the top of this file), so bumping the required sing-box version to v1.12.22 doesn’t actually change the code you build against and can be confusing. Either align the replace target/version with the intended upgrade, or keep the required version consistent with what’s actually used via replace.

Suggested change
github.com/sagernet/sing-box v1.12.22
github.com/sagernet/sing-box v1.12.19

Copilot uses AI. Check for mistakes.
myleshorton and others added 5 commits February 20, 2026 11:06
Forward cfg.BanditURLOverrides into the preTest urlTestOutbound so
bandit callback URLs are used during pre-start URL testing, not just
during normal operation.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Verify that URLOverrides from config.BanditURLOverrides are correctly
forwarded into the auto-lantern MutableURLTestOutboundOptions.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Periodically reports per-outbound throughput to the bandit system so it
can optimize for actual download speed, not just connectivity. Interval
and body size are randomized to avoid creating a fingerprintable signal.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@garmr-ulfr
Copy link
Collaborator

@myleshorton are the BanditURLOverrides specific to each proxy, meaning each new config will contain new override URLs? If that's the case, do they need to be? The existing overrides will become invalid once we receive a new config.

@myleshorton
Copy link
Contributor Author

@myleshorton are the BanditURLOverrides specific to each proxy, meaning each new config will contain new override URLs? If that's the case, do they need to be? The existing overrides will become invalid once we receive a new config.

Basically each config response is a test for each "arm" of the multi-armed bandit, so each URL has a token for that test that allows the server to identify it when the client makes the callback. So each proxy gets its own token essentially.

@garmr-ulfr
Copy link
Collaborator

garmr-ulfr commented Feb 24, 2026

Gotcha. Then they'll become invalid as soon as we receive a new config... We could add a field, (URLTester??), to MutableURLTestOptions to specify which tester implementation to use. Then we can implement a bandit-based tester that supports updating tokens dynamically when the config changes.

@myleshorton
Copy link
Contributor Author

I don't quite understand the problem sorry. What's bad about them becoming invalid?

@garmr-ulfr
Copy link
Collaborator

When a new config is received, won't it have new override URLs that are specific to the new proxies?

@myleshorton
Copy link
Contributor Author

Yeah, but that's what we want -- that means the server wants to test those.

@myleshorton
Copy link
Contributor Author

@garmr-ulfr
Copy link
Collaborator

Yeah, but that's what we want -- that means the server wants to test those.

Right, so when we get a new config while the VPN is connected, we update the outbounds/endpoints and just add the new tags to the MutableURLTest outbounds. Since they're not recreated, they won't have the new override URLs.

@myleshorton
Copy link
Contributor Author

Oh sorry I misunderstood! OK, I should be able to fix that.

Persist URLOverrides in servers.Options so they survive file-watcher
reloads, and call SetURLOverrides in updateGroup so new proxies from
a config refresh get their bandit callback URLs.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@myleshorton
Copy link
Contributor Author

OK that last commit implements a fairly straightforward solution I think?

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 8 out of 9 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

vpn/tunnel.go Outdated
Comment on lines 484 to 486
if err := t.mutGrpMgr.SetURLOverrides(autoTag, newOpts.URLOverrides); err != nil {
slog.Warn("Failed to set URL overrides", "group", autoTag, "error", err)
}
Copy link

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

newOpts.URLOverrides is used here after newOpts = removeDuplicates(...), but removeDuplicates currently rebuilds a fresh servers.Options without copying URLOverrides. That means overrides will be dropped and this call will usually set nil/empty overrides (effectively clearing them). Preserve the original overrides (e.g., stash before de-dupe and apply after) or update removeDuplicates to carry URLOverrides through.

Copilot uses AI. Check for mistakes.
Comment on lines 52 to +56
type Options struct {
Outbounds []option.Outbound `json:"outbounds,omitempty"`
Endpoints []option.Endpoint `json:"endpoints,omitempty"`
Locations map[string]C.ServerLocation `json:"locations,omitempty"`
Outbounds []option.Outbound `json:"outbounds,omitempty"`
Endpoints []option.Endpoint `json:"endpoints,omitempty"`
Locations map[string]C.ServerLocation `json:"locations,omitempty"`
URLOverrides map[string]string `json:"url_overrides,omitempty"`
Copy link

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

URLOverrides is added to servers.Options but Manager doesn't currently copy/persist it anywhere (e.g., setServers() rebuilds opts without copying it, Servers() doesn't clone it, and merge() doesn't merge it). As a result, any overrides passed into SetServers will be silently dropped and never reach the tunnel. Update the manager to store/clone/merge URLOverrides alongside the other fields (and ensure it round-trips via saveServers/loadServers).

Copilot uses AI. Check for mistakes.
@myleshorton myleshorton marked this pull request as draft February 26, 2026 00:11
- Fix URLOverrides lost through removeDuplicates: stash overrides
  before dedup and pass the stashed value to SetURLOverrides
- Persist URLOverrides through servers.Manager: clone in setServers
  and Servers so overrides survive save/load round-trips
- go.sum updated by go mod tidy

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants