Conversation
Adding check to ensure that device and manager atSigns are different Signed-off-by: Chris Swan <chris@atsign.com>
openwrt-ai
left a comment
openwrt-ai
left a comment
There was a problem hiding this comment.
Commit checks
- b74fbff "luci-app-csshnpd: warn if manager atSign same as device atSign" — the message covers only the equality check, but the diff also adds comma-separated multi-manager parsing (
writeManager), rejects commas in the device atSign, and adds input trimming infirstAt. Mention those in the body, or split them out.
Generated by Claude Code
| let list = value.split(',') | ||
| .map(s => s.trim()) | ||
| .filter(s => s.length > 0) | ||
| .map(s => s.startsWith('@') ? s : '@' + s); | ||
| value = list.join(','); |
There was a problem hiding this comment.
Accepting a comma-separated manager list is new behaviour, and the backend passes the option through unquoted as -m ${manager} in csshnpd.init:67 while the shipped default documents a single atSign (csshnpd.config:3). Does sshnpd -m really take a list? If so, the field description at config.js:131`` should stop saying "The manager atSign e.g. @manager".
Generated by Claude Code
There was a problem hiding this comment.
the field always accepted comma separated managers lists, it just did nothing specific to validate them. It's not a thing that we particularly encourage, hence keeping the singular 'manager' rather than plural 'managers'
| function normalizeAtsign(s) { | ||
| if (!s) | ||
| return ''; | ||
| s = s.trim().toLowerCase(); |
There was a problem hiding this comment.
nit: the comparison lowercases, but neither firstAt nor writeManager lowercases what is written, so @Device vs @device is rejected as a duplicate while mixed case still reaches UCI (and the init script derives a case-sensitive key path from it). Lowercase in the two write handlers as well if atSigns are meant to be case-insensitive.
Generated by Claude Code
There was a problem hiding this comment.
doesn't really matter what's in UCI as everything is flattened in the daemon (and users may wish to place capitals in atSigns for their own purposes)
2 participants
Pull request details
Description
Adding check to ensure that device and manager atSigns are different
Screenshot of changes
Maintainer
@cpswan
Tested on
OpenWrt version: OpenWrt 25.12.5 (r33051-f5dae5ece4)
LuCI version: LuCI openwrt-25.12 branch (26.239.42882~e60322b)
Web browser(s): Chrome 152.0.7977.82, Edge 152.0.4191.62
Checklist