Skip to content

luci-mod-system: improve Dropbear interface binding selection #8948

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Ser9ei wants to merge 1 commit into
base: master
Choose a base branch
from
Open

luci-mod-system: improve Dropbear interface binding selection #8948

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 29 additions & 6 deletions modules/luci-mod-system/htdocs/luci-static/resources/view/system/dropbear.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,39 @@ return view.extend({
o = s.option(form.Flag, 'enable', _('Enable Instance'), _('Enable <abbr title="Secure Shell">SSH</abbr> service instance'));
o.default = o.enabled;

o = s.option(form.Flag, '_direct', _('Bind to Interface'));
o.default = o.disabled;
// Virtual option: derives UI mode from Interface/DirectInterface,
// is not stored in UCI; inactive real options are removed on save
o = s.option(form.ListValue, '_bind_to', _('Bind to'), _('Select how the SSH service should be bound to network interfaces or IP addresses'));

@openwrt-ai openwrt-ai Aug 17, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

nit: the replaced _direct flag was actually persisted to /etc/config/dropbear, unlike the new _bind_to. form.Flag.parse() writes any non-default value (form.js:5153-5156) and uci.set() only filters option names starting with ., not _ (uci.js:607). So every instance where a user had ticked "Bind to Interface" carries a stale option _direct '1' that nothing reads any more (the dropbear init script only consumes Interface/DirectInterface), and this PR never removes it.

Would it be worth clearing it here, e.g. this.map.data.unset('dropbear', section, '_direct') alongside the this.remove(section) in the write handler below, so configs get tidied up on the next save?

Unrelated to that: the comment says the virtual option "clears the real options on change", but the clearing actually comes from the framework removing Interface/DirectInterface once their depends() stop matching — write() here only suppresses _bind_to itself.

Generated by Claude Code

o.widget = 'radio';
o.value('all', _('All interfaces (unspecified)'));
o.value('interface', _('IP addresses of interface'));
o.value('direct', _('Network interface'));
o.default = 'all';
o.cfgvalue = function(section) {
if (this.section.cfgvalue(section, 'DirectInterface'))
return 'direct';
if (this.section.cfgvalue(section, 'Interface'))
return 'interface';
return 'all';
};
o.forcewrite = true;
o.write = function(section) {
this.remove(section);
};

o = s.option(widgets.NetworkSelect, 'DirectInterface', _('Interface'), _('Listen only on the given interface or, if unspecified, on all'));
o = s.option(widgets.NetworkSelect, 'DirectInterface', _('Interface'), _('Listen only on the given interface'));
o.nocreate = true;
o.depends('_direct', '1');
o.depends('_bind_to', 'direct');
o.validate = function(section, value) {
return value ? true : _('Please select an interface');
};
Comment on lines +43 to +45

@openwrt-ai openwrt-ai Aug 17, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The dropdown still offers an explicit unspecified choice, which now duplicates the new All interfaces (unspecified) radio and can only ever produce this validation error. widgets.NetworkSelect adds that entry whenever rmempty or optional is set (widgets.js:507-508),`` and rmempty defaults to `true` (form.js:1431) — neither option sets it here.

Setting rmempty = false drops the entry from the choice list and makes the option required, so the custom validate() becomes redundant: parse() already rejects an empty active value with Option "Interface" must not be empty. (form.js:2152-2161). Clearing on mode switch is unaffected — an inactive option is removed via the !this.retain branch (form.js:2167-2169) regardless of rmempty.

Suggested change
o.validate = function(section, value) {
return value ? true : _('Please select an interface');
};
o.rmempty = false;

Same applies to lines 50-52 for Interface.

Generated by Claude Code


o = s.option(widgets.NetworkSelect, 'Interface', _('Interface'), _('Listen on up to 10 IPs on the given interface or, if unspecified, on all interfaces'));
o = s.option(widgets.NetworkSelect, 'Interface', _('Interface'), _('Listen on up to 10 IPs on the given interface'));
o.nocreate = true;
o.depends('_direct', '0');
o.depends('_bind_to', 'interface');
o.validate = function(section, value) {
return value ? true : _('Please select an interface');
};

o = s.option(form.Value, 'Port', _('Port'));
o.datatype = 'port';
Expand Down
24 changes: 24 additions & 0 deletions modules/luci-mod-system/root/etc/uci-defaults/90_luci-mod-system-dropbear
View file
Open in desktop

@openwrt-ai openwrt-ai Aug 18, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

nit: /etc/uci-defaults/ is a single flat namespace shared by every installed package, and this filename doesn't say which package owns it — it reads as if it belongs to the dropbear package, which already installs its own /etc/uci-defaults/50-dropbear from openwrt.git.

Every other uci-defaults file shipped by a LuCI package names its LuCI package: 50_luci-mod-admin-full, 40_luci-statistics, 95-luci-app-banip-housekeeping, 30_luci-theme-*, etc. Something like 90_luci-mod-system-dropbear would keep that pattern and make the owner obvious when the file shows up in luci-mod-system.list.

Generated by Claude Code

Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
#!/bin/sh

. /lib/functions.sh

# remove the "_direct" UI state from existing Dropbear configs

is_commit=0

dropbear_update() {
local _direct

config_get _direct "$1" _direct
[ -n "$_direct" ] || return 0

if uci -q delete dropbear."$1"._direct; then
is_commit=1
fi
}

config_load "dropbear"
config_foreach dropbear_update dropbear
[ $is_commit -eq 0 ] || uci commit dropbear
Comment on lines +7 to +22

@openwrt-ai openwrt-ai Aug 18, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

nit: the is_commit bookkeeping doesn't buy anything — both runners of uci-defaults scripts already uci commit unconditionally right after sourcing them: uci_apply_defaults() in /etc/init.d/boot on the boot path and default_postinst() on the package-install path. That's why the in-tree precedents for exactly this kind of stale-option cleanup just stage the delete and stop — 95-luci-app-attendedsysupgrade-housekeeping`` does uci -q delete with no commit at all, and openwrt's own `dropbear.defaults` relies on the same thing.

An unconditional uci -q commit dropbear is a no-op when the delta is empty, so the flag can go (also drops the unquoted $is_commit in [ ]). Using config_foreach is still right here, since s.addremove = true means there can be several dropbear sections.

Suggested change
is_commit=0
dropbear_update() {
local _direct
config_get _direct "$1" _direct
[ -n "$_direct" ] || return 0
if uci -q delete dropbear."$1"._direct; then
is_commit=1
fi
}
config_load "dropbear"
config_foreach dropbear_update dropbear
[ $is_commit -eq 0 ] || uci commit dropbear
dropbear_update() {
local _direct
config_get _direct "$1" _direct
[ -n "$_direct" ] || return 0
uci -q delete dropbear."$1"._direct
}
config_load "dropbear"
config_foreach dropbear_update dropbear
uci -q commit dropbear

Generated by Claude Code


exit 0