51ddns-agent: add package

[21hkcloud]

Description

Add the open-source client for the commercial, vendor-hosted 51DDNS remote-access service.

The package:

• builds Agent v0.6.3 from Apache-2.0 licensed Go source;
• uses the feed-provided frpc package instead of downloading a bundled binary;
• always runs the agent and its frpc children as the dedicated unprivileged ddns51 user with no_new_privs;
• enters a procd/ujail sandbox where /sbin/ujail is available, while small-flash targets without ujail remain supported and run unjailed but still unprivileged; and
• stores credentials with restricted permissions and preserves them on sysupgrade.

The service trust boundary is explicit: the vendor-hosted 51DDNS control plane assigns the authenticated FRP relay configuration, so users should install the client only if they trust the service operator.

LuCI and documentation

The LuCI interface is submitted independently, as required:
openwrt/luci#8958

English installation, UCI/LuCI configuration, verification, troubleshooting, removal, security and privacy documentation:
https://github.com/21hkcloud/51ddns-openwrt/blob/main/docs/openwrt.md

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

The v0.6.3 source archive does not contain luci-app-51ddns.

Verification

• The v0.6.3 codeload archive SHA-256 was independently verified as 102188815ddacc31777c47d2565921a002123632aa03004b4fb599c532ae9e0c.
• go test ./... passes on the upstream source.
• The OpenWrt and ImmortalWrt source-build matrix covers x86_64, aarch64_cortex-a53, mips_24kc, and mipsel_24kc.
• Runtime testing previously covered OpenWrt 25.12.5 on x86_64 and aarch64, and OpenWrt 24.10 on ramips/mt7621.

[21hkcloud]

Thanks. The reported issues have been corrected in the current head commit 2619ea1: the author and committer names are now set to Jinshuan Wang, and the package description is indented correctly. All three FormalityCheck jobs now pass.

…

On Wed, Aug 5, 2026 at 8:09 PM openwrt[bot] ***@***.***> wrote: *openwrt[bot]* left a comment (openwrt/packages#30158) <#30158 (comment)> Formality Check: Failed We checked this pull request against the contribution guidelines. Here is what needs your attention: 🛑 CRITICAL ERRORS *Commit 8934913 <https://github.com/openwrt/packages/commit/89349131def95f6935ed8676846e8d8bbd0cec8c>* - *51ddns-agent: add package*: - Author name format is invalid ('王金栓'). Please set your full name (first and last, e.g. 'Jane Doe'). - Committer name format is invalid ('王金栓'). Please set your full name (first and last, e.g. 'Jane Doe'). *Commit 8934913 <https://github.com/openwrt/packages/commit/89349131def95f6935ed8676846e8d8bbd0cec8c>* - *51ddns-agent: add package*: - Makefile line 'Securely connects an OpenWrt device to the self-hosted 51DDNS control plane.' inside 'Package/51ddns-agent/description' must be indented with at least 2 spaces Tip *Do not close this pull request* to make corrections. Instead, modify your existing commits (e.g. git commit --amend) and update the branch using git push --force-with-lease --force-if-includes. The checks will re-run automatically. ------------------------------ Something broken? Consider reporting an issue <https://github.com/openwrt/openwrt-bot-worker/issues/new>. *Running version 9016fac <openwrt/openwrt-bot-worker@9016fac> deployed on 2026-08-05 05:43:29 CEST* — Reply to this email directly, view it on GitHub <#30158?email_source=notifications&email_token=BONDHOBPZLPQRCQ52BFBP5D5IPZFDA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTKMJZHE4TGMZSG43KM4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2KYZTPN52GK4S7MNWGSY3L#issuecomment-5199933276>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/BONDHOBPY75VABIUPRDYA735IPZFDAVCNFSNUABEKJSXA33TNF2G64TZHMZDAMZQG44DGOB3JFZXG5LFHM2TANZXGE4DQMBQGWQXMAQ> . Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS <https://github.com/notifications/mobile/ios/BONDHOFUZQIYL4ZH46MJTMD5IPZFDA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTKMJZHE4TGMZSG43KM4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2KUZTPN52GK4S7NFXXG> and Android <https://github.com/notifications/mobile/android/BONDHOBFAJBQMXXKA4ELCXT5IPZFDA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTKMJZHE4TGMZSG43KM4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2K4ZTPN52GK4S7MFXGI4TPNFSA>. Download it today! You are receiving this because you authored the thread.Message ID: ***@***.***>

[BKPepe]

The PR description says "self-hosted 51DDNS control plane", but the README in your repo states Apache-2.0 only covers the client, and the default control_url points at api.51ddns.com. So in practice this is a client for a closed commercial cloud, nobody can self-host it. Please fix the description.

The bigger question: the agent pulls frpc configs from your API at runtime, meaning whoever operates api.51ddns.com controls what gets tunneled out of the user's network. Upstream is a repo created for this purpose (0 stars, 20 commits, no releases), so there is zero track record. On top of that the README, the website and the console are Chinese-only, so most users of this feed can't even read what the service does or what your privacy policy says before handing you a tunnel into their LAN. English docs are the bare minimum. Personally I'm not convinced the feed should carry a remote-access agent for an unknown commercial service at all.

Code-wise:

• everything runs as root, no procd user, no ujail. frpc doesn't need root for outbound tunnels, so why does the agent?
• validate_agent_section() is defined but never called
• extensions_enabled, webdav_enabled and remote_wakeup_enabled are shipped in the default config but the init script never touches them. Does the binary read UCI directly? WebDAV and remote wakeup also sound like considerably more than "DDNS", the package description should say what this thing actually does.
• the custom postinst can go, enable happens by default anyway and restarting on install is odd
• reload_service is just stop/start, so the reload trigger hard-restarts on every config change

PKG_MAINTAINER and Signed-off-by have to be a real person, "51DDNS support@51ddns.com" won't be accepted. And given the codex/ branch name this looks largely AI-generated, which is fine as such, but who is actually going to maintain this?

[openwrt-ai]

Reviewed 1 commit (2619ea1); the commit message matches the diff.

None of this blocks a merge outright — the formality checks are green, and the upstream v0.6.1 tag resolves (git ls-remote https://github.com/21hkcloud/51ddns-openwrt v0.6.1 → cb8c6ba). The two I would fix before merge are the /var/lib/51ddns directory shipped in the package payload (/var is a symlink to /tmp, so it never survives a reboot) and the custom postinst, which re-enables the service on every upgrade because it does not honour the PKG_UPGRADE guard that default_postinst applies. The remaining comments are dead code (validate_agent_section, restart_service, SERVICE_NAME), unused UCI options, and one question about whether multiple agent sections are meant to be supported — all optional.

---

Generated by Claude Code

[21hkcloud]

Thank you for the detailed review. I agree that the original wording and trust boundary were inaccurate.

I have force-pushed commit 1dd7972. The update:

• describes 51DDNS explicitly as a commercial, vendor-hosted service at api.51ddns.com and documents that the service operator supplies the runtime FRP configurations;
• adds an English README/trust model and an English privacy notice;
• changes PKG_MAINTAINER to Jinshuan Wang 21hkcloud@gmail.com;
• runs the Agent and its frpc children as the dedicated unprivileged ddns51 user with no_new_privs inside procd/ujail;
• removes /var/lib/51ddns from the package payload and creates runtime state during service startup;
• removes the custom postinst, redundant reload/restart code, unused UCI options, and multiple-section ambiguity; validation now applies to the single main section;
• uses a valid OpenWrt service account name after the SDK correctly rejected the earlier digit-prefixed name.

Validation is now green:

• all three OpenWrt FormalityCheck jobs pass;
• the full 8-job OpenWrt/ImmortalWrt source-build matrix passes for x86_64, aarch64_cortex-a53, mips_24kc, and mipsel_24kc: build run 31084340243;
• runtime install/upgrade, configuration preservation, restart, reboot, control-plane reconnect, and FRP login were tested on OpenWrt 25.12.5, ramips/mt7621.

Documentation and source:

• English README and trust model
• English privacy notice
• v0.6.2 source tag

The Agent is maintained by me, Jinshuan Wang, with company support from Shake Cloud Inc.

I understand the policy concern about a vendor-hosted remote-access service. If this category is out of scope for the feed regardless of hardening and disclosure, I will respect that and keep distribution in third-party feeds. Otherwise, I would appreciate another review of the corrected package.

[openwrt-ai]

Re-reviewed at 1dd7972 (force-push over 2619ea1). All seven findings from the previous round are addressed: /var/lib/51ddns is out of the payload, the custom postinst is gone, validate_agent_section/restart_service/SERVICE_NAME and the three unused UCI options are removed, the multi-section ambiguity is resolved into a single main section, and the description no longer claims "self-hosted". The upstream v0.6.2 tag resolves (git ls-remote → 3b8ae05) and the three FormalityCheck jobs are green.

The one I would fix before merge is the jail mount list: the agent execs /usr/bin/frpc, which is not mounted, so on an image with procd-ujail the tunnel cannot start. That also raises a question about the runtime test — procd-ujail is not in DEPENDS and procd_add_jail is used without requirejail, so a successful FRP login on the test device is consistent with the jail never having been applied. Worth confirming before the hardening claim in the commit message is taken at face value.

Secondary: the reload trigger was removed along with reload_service, so UCI changes no longer apply without a manual restart; uci_validate_section's return code is discarded; and the generated device identity under /etc/51ddns does not survive sysupgrade.

None of this speaks to @BKPepe's policy question about whether the feed should carry a vendor-hosted remote-access agent at all — that call is the maintainers'.

---

Generated by Claude Code

[21hkcloud]

Thanks for the re-review. Addressed in 2ecae04:

• require procd-ujail, add requirejail, and explicitly mount /usr/bin/51ddns-agent, /usr/bin/frpc, /bin/sleep, and /usr/bin/logger in the jail;
• restore procd_add_reload_trigger 51ddns without a custom reload_service, so procd can diff and reload the instance;
• abort startup when uci_validate_section fails;
• preserve /etc/51ddns/ across sysupgrade via /lib/upgrade/keep.d/51ddns-agent.

I also re-ran the jail path on OpenWrt 25.12.5 ramips/mt7621. service info reported the 51ddns-agent jail and the explicit mounts, ps showed the agent launched by /sbin/ujail, and the child frpc logged in successfully. The original test service was restored afterward.

All three FormalityCheck jobs pass. The OpenWrt/ImmortalWrt source-build matrix now passes all 8 targets (mipsel_24kc, mips_24kc, aarch64_cortex-a53, and x86_64 on both): https://github.com/21hkcloud/51ddns-openwrt/actions/runs/31096771374

[openwrt-ai]

Re-reviewed at 2ecae04 (force-push over 1dd7972). The commit message still matches the diff.

Correction to my previous round: the /bin/sh point I raised on the wrapper was wrong, and I should say so plainly. ujail runs add_path_and_deps() on the jail command and on every procd_add_jail_mount path, and that function detects a #! header and mounts the interpreter itself (add_script_interp() in jail/fs.c:464-485, reached from fs.c:544-546). /usr/libexec/51ddns-agent-start therefore pulls /bin/sh in on its own — which is why core inits jail #!/bin/sh scripts without mounting it. Same mechanism means /etc/passwd and /etc/group are added by ujail automatically once user/group is set (jail.c:2961-2963).

The mounts you did add are the ones that were genuinely missing: /usr/bin/51ddns-agent and /usr/bin/frpc are exec'd at runtime rather than being shebang/ELF deps ujail can discover, and /bin/sleep / /usr/bin/logger are busybox children of the wrapper. Bind-mounting those symlinks resolves through to /bin/busybox and busybox dispatches on argv[0], so they work as written. With log on the jail giving /dev/log, the mount list looks complete to me now.

The rest of last round is addressed as well: uci_validate_section failure now aborts startup, and service_triggers/procd_add_reload_trigger 51ddns is back — with no reload_service defined, rc.common's reload() falls through to start and lets procd diff the instance (rc.common:165-172). The keep.d drop-in follows the same install shape as openvpn/gnunet/privoxy, and /etc/51ddns/ as a directory entry is fine — list_static_conffiles feeds those lines straight to find (sysupgrade:167-171).

One new finding, inline: the +procd-ujail dependency takes the package off every small_flash target. Nothing here blocks a merge on its own.

The config_load 51ddns on line 19 is still dead (validate_data reads UCI itself) — cosmetic, not worth a force-push by itself.

FormalityCheck is green on 2ecae04. As before, none of this touches @BKPepe's policy question about whether the feed should carry a vendor-hosted remote-access agent at all; that remains the maintainers' call.

---

Generated by Claude Code

[openwrt-ai]

Re-reviewed at 40f8e2e (force-push over 2ecae04). Three lines changed, all of them last round's finding: +procd-ujail is out of DEPENDS, requirejail is off procd_add_jail, and the dead config_load 51ddns is gone.

That resolves the small_flash exclusion. I checked that nothing else re-introduces the transitive KERNEL_NAMESPACES dependency — frpc carries only $(GO_ARCH_DEPENDS) (frp/Makefile:58) and ca-bundle has no config symbols — so the package is selectable on ramips/mt76x8, ath79/tiny and friends again, which is the install base the mips test matrix speaks to. The remaining posture (jail applied wherever /sbin/ujail exists, service unjailed but still unprivileged elsewhere) is what core does: dnsmasq.init:1277 jails without depending on procd-ujail, and sysntpd:73 guards its jail block on the binary being present.

Dropping config_load 51ddns is correct: uci_validate_section goes through /sbin/validate_data, which reads UCI itself, and nothing else in the script uses config_get/config_foreach any more.

The commit message still matches the diff, and was updated for the softened jail claim. No new issues; one wording nit inline about the PR body, which still describes the jail as unconditional.

FormalityCheck reports nothing failing on 40f8e2e. As in previous rounds, none of this addresses @BKPepe's policy question — whether the feed should carry a client for a vendor-hosted remote-access service at all — and the NAK label suggests that is where the PR actually stands. That call is the maintainers'.

---

Generated by Claude Code

[21hkcloud]

Hello, just a polite follow-up on the remaining policy question.

The technical findings have been addressed, the FormalityCheck jobs remain green, and the client, trust model, privacy notice, and vendor-hosted nature of the service are now documented explicitly. I understand and respect that inclusion of a client for a commercial vendor-hosted remote-access service is ultimately a feed policy decision.

When convenient, could a maintainer please confirm whether this package category is considered out of scope for the official feed, or whether the corrected package may proceed to final review? A clear decision either way would help us choose the appropriate long-term distribution channel. Thank you.

[BKPepe]

Looking at this again, you currently have two open pull requests: this one and #30176. They are essentially the same thing, and looking at both websites, they appear visually similar and AI-generated. The only difference being that one supports English while the other is entirely in Chinese with no translation options.

At the same time, @openwrt-ai keeps pointing this out:

As before, none of this touches @BKPepe's policy question about whether the feed should carry a vendor-hosted remote-access agent at all

Frankly, I am quite strongly against adding a package when there is no LuCI interface, no documentation, and the website is incomprehensible to me. Furthermore, luci-app-51ddns in your repository belongs in openwrt/luci and should not be bundled inside the tarball being downloaded.

So we essentially have a package, but no way of knowing how to actually use it.

Therefore, my NAK still stands and nothing has changed.

[21hkcloud]

Thank you for the direct feedback. I have now addressed each concrete issue:

• closed the duplicate 51SDWAN package PR (51sdwan: add OpenWrt edge agent #30176) to keep this review focused;
• removed luci-app-51ddns from the v0.6.3 agent source archive;
• submitted the LuCI application independently as luci-app-51ddns: add remote access agent interface luci#8958;
• published English installation, configuration, trust-model, privacy, troubleshooting and removal documentation at https://www.51ddns.com/openwrt;
• updated this package to v0.6.3 and restored all FormalityCheck jobs to green.

The package and documentation now make the vendor-hosted nature and relay trust boundary explicit. I understand that the remaining question is the feed policy decision about carrying this class of client, and I will respect the maintainers' decision. If there are further technical issues in either PR, I will address them there.

[openwrt-ai]

Re-reviewed at 3a2b025 (one new commit on top of 40f8e2e). The diff is two lines — PKG_VERSION and PKG_HASH — and the commit message matches it.

I checked the claim the bump rests on. The v0.6.3 tag resolves (git ls-remote https://github.com/21hkcloud/51ddns-openwrt v0.6.3 → tag 23058f0, commit c94576f), and luci-app-51ddns/ is indeed gone from that tree, so @BKPepe's point about the LuCI app being bundled in the downloaded tarball is addressed at the source. Comparing v0.6.2 and v0.6.3 end to end, the only differences are the removed LuCI app, a README edit, and the upstream repo's own copy of the OpenWrt packaging — cmd/51ddns-agent/main.go is byte-identical between the two tags. So the built binary does not change, which is consistent with the commit message describing this purely as an archive-contents change.

Packaging side-effects of the bump are all fine: PKG_RELEASE correctly stays at 1 on a version update, PKG_BUILD_DIR and PKG_SOURCE derive from PKG_VERSION so they follow, go.mod still declares no external requires (no vendoring needed), and GO_PKG_LDFLAGS_X:=main.agentVersion=$(PKG_VERSION) keeps --version printing 51ddns-agent 0.6.3, which is what test.sh and the generic CI version check compare against.

One thing I could not verify: PKG_HASH. codeload.github.com is not reachable from this session, so I have no independent confirmation that 1021888... matches the v0.6.3 tarball. Worth a make package/51ddns-agent/download check before merge, since a git tag is mutable and this is the only line nothing else cross-checks.

Two nits inline, neither blocking: the introduce-then-bump commit pair, and a wording regression in the PR body about the jail being unconditional (a caveat that was added in an earlier round and lost in the rewrite).

FormalityCheck is green on 3a2b025; no build job has run at this head. As in every previous round, none of the above speaks to the policy question — whether the feed should carry a client for a vendor-hosted remote-access service at all — which is where the NAK label puts this PR. That call is the maintainers'.

---

Generated by Claude Code

[21hkcloud]

Technical follow-up completed:

• Squashed the package submission to a single 51ddns-agent: add package commit (306cde7).
• Restored the optional ujail / small-flash caveat: the service always runs as the unprivileged ddns51 user with no_new_privs, and enters ujail only when /sbin/ujail is available.
• Independently verified the v0.6.3 codeload archive hash: 102188815ddacc31777c47d2565921a002123632aa03004b4fb599c532ae9e0c.
• Added English OpenWrt installation, UCI/LuCI configuration, verification, troubleshooting, removal, security, trust-boundary and privacy documentation: https://github.com/21hkcloud/51ddns-openwrt/blob/main/docs/openwrt.md
• Source tests pass (go test ./...).
• The 0.6.3 source-build matrix passed for OpenWrt and ImmortalWrt on x86_64, aarch64_cortex-a53, mips_24kc and mipsel_24kc: https://github.com/21hkcloud/51ddns-openwrt/actions/runs/32439196504
• Downloaded and inspected all eight artifacts; every agent package is 51ddns-agent-0.6.3-r1.apk.
• The separate LuCI submission remains open as luci-app-51ddns: add remote access agent interface luci#8958.

I understand that the remaining NAK is policy-level rather than a currently unresolved build/formality issue. Could the maintainers please clarify whether vendor-hosted remote-access agents are categorically out of scope for the packages feed, or whether the technical, documentation and packaging concerns are now satisfied for further review?

[openwrt-ai]

Re-reviewed at 306cde7 (force-push over 3a2b025).

git diff 3a2b025..306cde7 is empty — the tree is byte-identical to the head I reviewed last round. The change is history-only: the two commits (40f8e2e introducing the package at 0.6.2 and 3a2b025 bumping to 0.6.3) are now a single 51ddns-agent: add package at 0.6.3, authored and committed as Jinshuan Wang. That closes the introduce-then-bump nit from last round, and the squashed message reads correctly against the diff: it describes the v0.6.3 build, the unprivileged ddns51 user with no_new_privs, the best-effort ujail ("enters an ujail where the target provides it"), continued small-flash support, and the separate openwrt/luci submission — all of which the six added files actually do.

The second nit is closed too: the PR body now carries the caveat again ("enters a procd/ujail sandbox where /sbin/ujail is available, while small-flash targets without ujail remain supported and run unjailed but still unprivileged"), which matches DEPENDS having no +procd-ujail and procd_add_jail having no requirejail.

Since no code changed, the findings from earlier rounds stand as previously resolved and there is nothing new to raise. PKG_HASH remains the one line I still cannot independently confirm — codeload.github.com is not reachable from this session — so a make package/51ddns-agent/download before merge is still worth it, since a git tag is mutable.

All three FormalityCheck jobs are green on 306cde7; no build job has run at this head.

As in every previous round, none of this speaks to @BKPepe's policy question — whether the feed should carry a client for a vendor-hosted remote-access service at all — which is where the NAK label puts this PR. That call is the maintainers'.

---

Generated by Claude Code