Skip to content

luci-app-51ddns: add remote access agent interface - #8958

Open
21hkcloud wants to merge 1 commit into
openwrt:masterfrom
21hkcloud:codex/luci-app-51ddns
Open

luci-app-51ddns: add remote access agent interface#8958
21hkcloud wants to merge 1 commit into
openwrt:masterfrom
21hkcloud:codex/luci-app-51ddns

Conversation

@21hkcloud

Copy link
Copy Markdown

Description

Add a LuCI interface for the open-source 51DDNS OpenWrt agent.

The page is available under Services > 51DDNS Remote Access and provides:

  • account-token configuration with password masking;
  • service status and installed agent version;
  • current plan name, expiry and remaining time;
  • a direct link to the 51DDNS console.

The application depends on 51ddns-agent, which is submitted separately in openwrt/packages#30158. The LuCI application is intentionally not bundled in the agent source archive.

Documentation and trust model

English installation, operation, privacy, troubleshooting and removal documentation:
https://www.51ddns.com/openwrt

Agent source and client privacy notice:
https://github.com/21hkcloud/51ddns-openwrt

The UI explicitly identifies 51DDNS as a vendor-hosted remote-access service. The agent uses the feed-provided frpc package and runs as a dedicated unprivileged user in a procd/ujail sandbox.

Testing

  • The page and RPC helper use translatable English strings.
  • ACL access is limited to the 51DDNS UCI configuration, service status and the dedicated read-only RPC helper.
  • The app has been tested with the source-built agent on OpenWrt 24.10 and 25.12 snapshots.

@openwrt openwrt Bot added add package Introduces a new package Makefile build script not following guidelines Pull request does not follow formatting guidelines labels Aug 20, 2026
@21hkcloud
21hkcloud force-pushed the codex/luci-app-51ddns branch from 39a4377 to 1b84021 Compare August 20, 2026 14:09
Add an English LuCI page for configuring the 51DDNS account token.

The page also reports the agent service state, version, plan and expiry.

Signed-off-by: Jinshuan Wang <21hkcloud@gmail.com>
@21hkcloud
21hkcloud force-pushed the codex/luci-app-51ddns branch from 1b84021 to 746fff9 Compare August 20, 2026 14:10
@openwrt openwrt Bot removed the not following guidelines Pull request does not follow formatting guidelines label Aug 20, 2026

@openwrt-ai openwrt-ai left a comment

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.

Reviewed 1 new commit; see inline comments. The commit message matches its changes, and the backend dependency on 51ddns-agent (openwrt/packages#30158) is already called out in the PR description, so I did not flag it as a missing consumer.


Generated by Claude Code

Comment on lines +20 to +23
info)
version="$(/usr/bin/51ddns-agent --version 2>/dev/null | head -n 1)"
printf '{"version":"%s"}\n' "$(json_escape "$version")"
;;

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 hand-rolled json_escape only escapes \ and ". Any control character in the agent's --version output — a tab, a CR from a \r\n line ending, a backspace — is emitted raw into the string and produces invalid JSON, which makes the whole info ubus call fail rather than just showing a mangled version string. head -n 1 strips the LF but not a trailing CR.

Other rpcd helpers in this repo use jshn for exactly this reason, e.g. luci.squid and olsrvizinfo.

Suggested change
info)
version="$(/usr/bin/51ddns-agent --version 2>/dev/null | head -n 1)"
printf '{"version":"%s"}\n' "$(json_escape "$version")"
;;
info)
version="$(/usr/bin/51ddns-agent --version 2>/dev/null | head -n 1)"
json_init
json_add_string version "$version"
json_dump
;;

This also needs . /usr/share/libubox/jshn.sh near the top of the file, and the json_escape helper on lines 3-5 can then be dropped.


Generated by Claude Code

Comment on lines +13 to +19
status)
if [ -s /var/lib/51ddns/status.json ]; then
cat /var/lib/51ddns/status.json
else
echo '{"online":false}'
fi
;;

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.

status relays the whole of /var/lib/51ddns/status.json verbatim as the ubus reply, but the view only consumes plan.product_name and plan.expires_at. Two consequences worth confirming:

  1. Everything the agent happens to write into that file becomes readable by any session holding the luci-app-51ddns read ACL. Can status.json contain the account token, a device secret or a relay credential? If so, this widens the exposure well beyond what the page needs — emitting only the fields the view reads (online, plan.product_name, plan.expires_at) would keep the ACL honest.
  2. The file is passed through without any validation, so if the agent rewrites it non-atomically a partial read is returned as the ubus payload and the call fails to parse instead of falling back to {"online":false}.

Generated by Claude Code

Comment on lines +62 to +65
const hours = Math.ceil(milliseconds / 3600000);
const label = hours > 48
? _('About %d days').format(Math.ceil(hours / 24))
: _('%d hours').format(hours);

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: these two strings are count-dependent, so translators into languages with more than two plural forms can't render them correctly through _(). luci-base exposes N_(n, singular, plural) for this (see nftables.js:401),`` and it is already whitelisted as a global in eslint.config.mjs.

Suggested change
const hours = Math.ceil(milliseconds / 3600000);
const label = hours > 48
? _('About %d days').format(Math.ceil(hours / 24))
: _('%d hours').format(hours);
const hours = Math.ceil(milliseconds / 3600000);
const days = Math.ceil(hours / 24);
const label = hours > 48
? N_(days, 'About %d day', 'About %d days').format(days)
: N_(hours, '%d hour', '%d hours').format(hours);

The .pot template needs regenerating afterwards so the entries pick up their msgid_plural form.


Generated by Claude Code

Comment on lines +96 to +98
const section = map.section(form.NamedSection, 'main', 'agent', _('Quick setup'));
section.anonymous = true;
section.addremove = false;

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: anonymous is only consulted by TypedSection/GridSection; NamedSection never reads it, so line 97 has no effect.

Suggested change
const section = map.section(form.NamedSection, 'main', 'agent', _('Quick setup'));
section.anonymous = true;
section.addremove = false;
const section = map.section(form.NamedSection, 'main', 'agent', _('Quick setup'));
section.addremove = false;

Separately: with addremove = false this page can only edit a pre-existing config agent 'main' section — if /etc/config/51ddns is missing or has no such section the form renders empty and the user has no way to enter the token. This package ships no root/etc/config/51ddns, so I assume 51ddns-agent provides it with that exact section name; can you confirm that's the case in openwrt/packages#30158?


Generated by Claude Code

Comment on lines +13 to +23
#: applications/luci-app-51ddns/htdocs/luci-static/resources/view/51ddns/overview.js:133
msgid "Account token"
msgstr ""

#: applications/luci-app-51ddns/htdocs/luci-static/resources/view/51ddns/overview.js:105
msgid "Agent version"
msgstr ""

#: applications/luci-app-51ddns/htdocs/luci-static/resources/view/51ddns/overview.js:64
msgid "About %d days"
msgstr ""

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: "About %d days" sorts before "Account token", so this template can't have come out of the in-tree scanner — it pipes everything through msguniq -s, which emits msgids in strict byte order. Please regenerate the template with the repo's i18n scanner rather than hand-maintaining it, otherwise the next automated refresh will churn this file.

Suggested change
#: applications/luci-app-51ddns/htdocs/luci-static/resources/view/51ddns/overview.js:133
msgid "Account token"
msgstr ""
#: applications/luci-app-51ddns/htdocs/luci-static/resources/view/51ddns/overview.js:105
msgid "Agent version"
msgstr ""
#: applications/luci-app-51ddns/htdocs/luci-static/resources/view/51ddns/overview.js:64
msgid "About %d days"
msgstr ""
#: applications/luci-app-51ddns/htdocs/luci-static/resources/view/51ddns/overview.js:64
msgid "About %d days"
msgstr ""
#: applications/luci-app-51ddns/htdocs/luci-static/resources/view/51ddns/overview.js:133
msgid "Account token"
msgstr ""
#: applications/luci-app-51ddns/htdocs/luci-static/resources/view/51ddns/overview.js:105
msgid "Agent version"
msgstr ""

(If you take the N_() change suggested on overview.js, ignore this hunk and just regenerate — the entries change shape anyway.)


Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

add package Introduces a new package Makefile build script

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants