Add --interface option to bind to a specific network interface#235
Add --interface option to bind to a specific network interface#235
Conversation
Agent-Logs-Url: https://github.com/Sendspin/sendspin-cli/sessions/522a5953-761f-4970-975f-ce335a6bb318 Co-authored-by: balloob <1444314+balloob@users.noreply.github.com>
…ne check Agent-Logs-Url: https://github.com/Sendspin/sendspin-cli/sessions/522a5953-761f-4970-975f-ce335a6bb318 Co-authored-by: balloob <1444314+balloob@users.noreply.github.com>
| sendspin servers list | ||
| ``` | ||
|
|
||
| ### Network Interface Binding |
There was a problem hiding this comment.
@copilot move this to bottom of option descriptions ni README, it is not important
There was a problem hiding this comment.
Done in 404586e — moved the "Network Interface Binding" section to just before the "Install as Daemon" heading.
Agent-Logs-Url: https://github.com/Sendspin/sendspin-cli/sessions/5532e42c-d698-4805-8829-188854b9e876 Co-authored-by: balloob <1444314+balloob@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds a new --interface <IP> option (and corresponding persisted setting) to restrict Sendspin’s network behavior on multi-homed machines, primarily to avoid exposing discovery/listeners on unintended interfaces.
Changes:
- Introduces
interface: str | NoneinClientSettingsand loads it from the settings JSON. - Threads an optional
interfaceslist through mDNS discovery (ServiceDiscovery,discover_servers(),discover_clients()) and uses it when creatingAsyncZeroconf. - Adds
--interfaceto the player and daemon CLIs, passes it throughAppArgs/DaemonArgs, and binds the daemon’s server-initiated listener to that address.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| sendspin/tui/app.py | Passes a single interface (when configured) into ServiceDiscovery for mDNS discovery scoping. |
| sendspin/settings.py | Adds persisted interface setting and loads it from JSON. |
| sendspin/discovery.py | Adds interface scoping support to ServiceDiscovery, discover_servers(), and discover_clients() via AsyncZeroconf(interfaces=...). |
| sendspin/daemon/daemon.py | Binds the server-initiated listener to the configured interface (else 0.0.0.0). |
| sendspin/cli.py | Adds --interface for player + daemon, applies settings defaults, and passes through to args structs. |
| README.md | Documents the new interface setting and adds a “Network Interface Binding” section. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if args.interface is None: | ||
| args.interface = settings.interface |
There was a problem hiding this comment.
args.interface is accepted from both CLI and persisted settings, but it's never validated. If a user provides an invalid IP (or a hostname/interface name that aiohttp/zeroconf can't bind to), this can raise an OSError during listener startup and currently bubbles up to an unhandled exception in main(). Consider validating --interface early (e.g., with ipaddress.ip_address) and turning failures into a CLIError with a clear message.
| interfaces: Optional list of IP addresses or interface names to restrict | ||
| mDNS discovery to. If None, all interfaces are used. |
There was a problem hiding this comment.
The new interfaces parameter is documented as accepting "IP addresses or interface names", but the CLI/README describe --interface as an IP address only. Please align the documentation and supported inputs (either document/validate interface names everywhere, or narrow this docstring to IP addresses) so users know what is actually supported.
| interfaces: Optional list of IP addresses or interface names to restrict | |
| mDNS discovery to. If None, all interfaces are used. | |
| interfaces: Optional list of IP addresses to restrict mDNS | |
| discovery to. If None, all interfaces are used. |
| self.manufacturer = data.get("manufacturer") | ||
| self.product_name = data.get("product_name") | ||
| self.last_played_server_id = data.get("last_played_server_id") | ||
| self.interface = data.get("interface") |
There was a problem hiding this comment.
ClientSettings now persists an interface field, but there isn't corresponding unit coverage. Since there are already tests asserting settings load behavior for other fields, it would be good to add a small test that writes {"interface": "192.168.1.5"} and asserts it is loaded (and/or that to_dict() includes it on save).
| help=( | ||
| "IP address of the network interface to bind to. " | ||
| "In server-initiated mode (no --url), restricts the listening server to this " | ||
| "interface only. Also restricts mDNS discovery to this interface. " |
There was a problem hiding this comment.
The daemon subcommand's --interface help text says it "also restricts mDNS discovery", but the daemon code path doesn't use ServiceDiscovery/discover_*() at all. As implemented, --interface only affects the incoming listener bind address (and is ignored in client-initiated mode). Please adjust the help text to match the actual behavior to avoid misleading users.
| "interface only. Also restricts mDNS discovery to this interface. " | |
| "interface only. Ignored in client-initiated mode (--url). " |
Users on multi-homed machines (e.g., a home server acting as a LAN router) have no way to prevent Sendspin from being accessible on all interfaces, including WAN-facing ones. This adds a
--interface <IP>option to restrict mDNS discovery and daemon listener binding to a single interface.Changes
settings.py: Addedinterface: str | NonetoClientSettingswith full load/save/update support.discovery.py:ServiceDiscoveryacceptsinterfaces: list[str] | None; passes it toAsyncZeroconfto restrict mDNS to that interface. Same parameter threaded throughdiscover_servers()anddiscover_clients().cli.py:--interfaceadded to both TUI player and daemon argument parsers; settings default applied; passed through toAppArgs/DaemonArgs.tui/app.py:AppArgsgainsinterface;SendspinApppasses[interface]toServiceDiscovery.daemon/daemon.py:DaemonArgsgainsinterface; server-initiated listener useshost=interface(falls back to0.0.0.0) so the aiohttp server only binds on that IP.README.md: New "Network Interface Binding" section;interfaceadded to settings table and example config.Usage
Can also be persisted in
~/.config/sendspin/settings-daemon.json:{ "interface": "192.168.1.5" }Warning
Firewall rules blocked me from connecting to one or more addresses (expand for details)
I tried to connect to the following addresses, but was blocked by firewall rules:
python-zeroconf.readthedocs.io/home/REDACTED/work/_temp/ghcca-node/node/bin/node /home/REDACTED/work/_temp/ghcca-node/node/bin/node --enable-source-maps /home/REDACTED/work/_temp/copilot-developer-action-main/dist/index.js(dns block)If you need me to access, download, or install something from one of these locations, you can either: