ddns-scripts: add namesilo.com #28095

cdfmr · 2025-12-16T09:10:31Z

📦 Package Details

Maintainer: @

Description:
Add namesilo.com API to ddns-scripts package.

🧪 Run Testing Details

OpenWrt Version: r32320
OpenWrt Target/Subtarget: MediaTek/Filogic
OpenWrt Device: Cudy TR3000 v1

✅ Formalities

I have reviewed the CONTRIBUTING.md file for detailed contributing guidelines.

GeorgeSapkin · 2025-12-16T12:55:05Z

You need to add a commit message, and use your real name both for author and sign-off, which is missing as per the contributing guidelines.

GeorgeSapkin

And you need to bump the PKG_RELEASE in the Makefile.

GeorgeSapkin · 2025-12-16T12:58:59Z

net/ddns-scripts/Makefile

+
+

Suggested change

GeorgeSapkin · 2025-12-16T12:59:02Z

net/ddns-scripts/Makefile

+
+

Suggested change

GeorgeSapkin · 2025-12-16T12:59:07Z

net/ddns-scripts/files/usr/lib/ddns/update_namesilo_com_v1.sh

@@ -0,0 +1,28 @@
+#
+#.Distributed under the terms of the GNU General Public License (GPL) version 2.0


Consider using a SPDX license identifier instead.

GeorgeSapkin · 2025-12-16T12:59:09Z

net/ddns-scripts/files/usr/lib/ddns/update_namesilo_com_v1.sh

+}
+
+# update subdomain record
+rrid=`call_api dnsListRecords "domain=$domain" | get_rrid`


Suggested change

rrid=`call_api dnsListRecords "domain=$domain" | get_rrid`

rrid=$(call_api dnsListRecords "domain=$domain" | get_rrid)

GeorgeSapkin · 2025-12-16T12:59:11Z

net/ddns-scripts/files/usr/lib/ddns/update_namesilo_com_v1.sh

+call_api dnsUpdateRecord "domain=$domain&rrid=$rrid&rrhost=$username&rrvalue=$__IP&rrttl=$__TTL" | grep success && {
+	return 0
+}


Did you mean:

Suggested change

call_api dnsUpdateRecord "domain=$domain&rrid=$rrid&rrhost=$username&rrvalue=$__IP&rrttl=$__TTL" | grep success && {

return 0

}

call_api dnsUpdateRecord "domain=$domain&rrid=$rrid&rrhost=$username&rrvalue=$__IP&rrttl=$__TTL" | grep success && true

cdfmr · 2025-12-17T08:41:58Z

Updated with all suggestions.

GeorgeSapkin · 2025-12-17T13:38:21Z

You still need to add a commit message with a description of the changes, besides the subject.

GeorgeSapkin · 2025-12-19T01:39:15Z

net/ddns-scripts/files/usr/lib/ddns/update_namesilo_com_v1.sh

@@ -0,0 +1,24 @@
+#
+# SPDX-License-Identifier: GPL-2.0


There's no such identifier. Did you mean GPL-2.0-only?

https://spdx.org/licenses/

Yes, should I update the file and re-commit?

Yes, please.

new service provider namesilo.com config guide: * set [domain] to apex domain * set [username] to subdomain (without apex domain) * set [password] to api key Signed-off-by: Lin Fan <im.linfan@gmail.com>

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 12 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-26T09:19:39Z

net/ddns-scripts/files/usr/lib/ddns/update_namesilo_com_v1.sh

+__TTL=3600
+
+# wrap some routines
+call_api() {


Security concern: The API key (password) is passed directly in the URL query string. This could expose the API key in process listings (ps command), shell history, and system logs. While this may be the only method supported by the NameSilo API, consider adding a comment noting this security consideration, similar to how other API scripts document their authentication methods.

Suggested change

call_api() {

call_api() {

# SECURITY NOTE:

# The NameSilo API requires the API key to be passed as a URL query parameter.

# This means the key may be exposed in shell history, process listings (ps),

# and system logs that record full command lines. Use a dedicated API key and

# protect access to this system and its logs accordingly.

Copilot · 2025-12-26T09:19:40Z

net/ddns-scripts/files/usr/lib/ddns/update_namesilo_com_v1.sh

+# variable __IP already defined with the ip-address to use for update
+#
+
+__TTL=3600


The __TTL variable is hardcoded to 3600 seconds but is not sourced from any configuration option. Other scripts in this project typically allow TTL to be configured via param_opt or use a configurable default. Consider documenting this limitation or making the TTL configurable.

Suggested change

__TTL=3600

# TTL (seconds); can be overridden by param_opt_ttl if provided by ddns config

__TTL="${param_opt_ttl:-3600}"

Copilot · 2025-12-26T09:19:40Z

net/ddns-scripts/files/usr/lib/ddns/update_namesilo_com_v1.sh

@@ -0,0 +1,24 @@
+#


Missing shebang line. All shell scripts in this project start with '#!/bin/sh'. Other update scripts like update_godaddy_com_v1.sh include this on line 1.

Copilot · 2025-12-26T09:19:40Z

net/ddns-scripts/files/usr/lib/ddns/update_namesilo_com_v1.sh

+
+# update subdomain record
+rrid=$(call_api dnsListRecords "domain=$domain" | get_rrid)
+call_api dnsUpdateRecord "domain=$domain&rrid=$rrid&rrhost=$username&rrvalue=$__IP&rrttl=$__TTL" | grep success


Using 'grep success' without checking the exit status doesn't provide proper error handling. The script should verify that the update was successful and use write_log to report the result. Additionally, the exit status of this line determines the success of the entire script, but there's no explicit error handling or return value check.

Suggested change

call_api dnsUpdateRecord "domain=$domain&rrid=$rrid&rrhost=$username&rrvalue=$__IP&rrttl=$__TTL" | grep success

if call_api dnsUpdateRecord "domain=$domain&rrid=$rrid&rrhost=$username&rrvalue=$__IP&rrttl=$__TTL" | grep -q success; then

write_log 7 "Namesilo: DNS record for '$username.$domain' updated successfully to IP '$__IP'"

else

write_log 3 "Namesilo: Failed to update DNS record for '$username.$domain' to IP '$__IP'"

return 1

fi

Copilot · 2025-12-26T09:19:41Z

net/ddns-scripts/files/usr/lib/ddns/update_namesilo_com_v1.sh

+	wget -q -O- "https://www.namesilo.com/api/$1?version=1&type=xml&key=$password&$2"
+}
+get_rrid() {
+	grep -o "<record_id>.*$username</host>" | sed 's/.*<record_id>//g;s/<.*//g'
+}
+
+# update subdomain record
+rrid=$(call_api dnsListRecords "domain=$domain" | get_rrid)
+call_api dnsUpdateRecord "domain=$domain&rrid=$rrid&rrhost=$username&rrvalue=$__IP&rrttl=$__TTL" | grep success


The script does not utilize the standard DATFILE and ERRFILE variables that are used throughout the ddns-scripts framework. Instead, it redirects output directly via '-O-', which makes debugging difficult and doesn't integrate with the framework's error reporting mechanisms. See update_godaddy_com_v1.sh for proper usage of these variables.

Suggested change

wget -q -O- "https://www.namesilo.com/api/$1?version=1&type=xml&key=$password&$2"

}

get_rrid() {

grep -o "<record_id>.*$username</host>" | sed 's/.*<record_id>//g;s/<.*//g'

}

# update subdomain record

rrid=$(call_api dnsListRecords "domain=$domain" | get_rrid)

call_api dnsUpdateRecord "domain=$domain&rrid=$rrid&rrhost=$username&rrvalue=$__IP&rrttl=$__TTL" | grep success

wget -q -O "$DATFILE" "https://www.namesilo.com/api/$1?version=1&type=xml&key=$password&$2" 2>"$ERRFILE"

}

get_rrid() {

grep -o "<record_id>.*$username</host>" | sed 's/.*<record_id>//g;s/<.*//g'

}

# update subdomain record

call_api dnsListRecords "domain=$domain"

rrid=$(get_rrid <"$DATFILE")

call_api dnsUpdateRecord "domain=$domain&rrid=$rrid&rrhost=$username&rrvalue=$__IP&rrttl=$__TTL"

grep success "$DATFILE"

Copilot · 2025-12-26T09:19:42Z

net/ddns-scripts/files/usr/lib/ddns/update_namesilo_com_v1.sh

+rrid=$(call_api dnsListRecords "domain=$domain" | get_rrid)
+call_api dnsUpdateRecord "domain=$domain&rrid=$rrid&rrhost=$username&rrvalue=$__IP&rrttl=$__TTL" | grep success


The script does not differentiate between IPv4 (A records) and IPv6 (AAAA records) updates. The use_ipv6 variable is not checked to determine the record type. While the __IP variable will contain the correct IP format, the NameSilo API may require explicit record type specification. Check the NameSilo API documentation and consider adding record type handling similar to other scripts like update_digitalocean_com_v2.sh (line 27).

Copilot · 2025-12-26T09:19:42Z

net/ddns-scripts/files/usr/lib/ddns/update_namesilo_com_v1.sh

+#
+# variable __IP already defined with the ip-address to use for update
+#
+


Missing input validation. The script should verify that required parameters (username, password, domain) are not empty using write_log 14 error checking, consistent with other update scripts in this project. For example, update_godaddy_com_v1.sh checks these parameters on lines 22-23.

Suggested change

[ -z "$username" ] && write_log 14 "ERROR: 'username' (sub domain) not set" && return 1

[ -z "$password" ] && write_log 14 "ERROR: 'password' (api key) not set" && return 1

[ -z "$domain" ] && write_log 14 "ERROR: 'domain' not set" && return 1

Copilot · 2025-12-26T09:19:42Z

net/ddns-scripts/files/usr/lib/ddns/update_namesilo_com_v1.sh

+
+# wrap some routines
+call_api() {
+	wget -q -O- "https://www.namesilo.com/api/$1?version=1&type=xml&key=$password&$2"


Missing error handling. The call_api function does not check the exit status of wget or validate the API response. If wget fails or returns an error, the script will continue execution and potentially pass invalid data to subsequent commands. Other scripts in this project properly check command exit status and use write_log for error reporting.

Suggested change

wget -q -O- "https://www.namesilo.com/api/$1?version=1&type=xml&key=$password&$2"

local endpoint="$1"

local params="$2"

local url="https://www.namesilo.com/api/${endpoint}?version=1&type=xml&key=${password}&${params}"

local response

response="$(wget -q -O- "$url")"

local rc=$?

if [ "$rc" -ne 0 ]; then

write_log 3 "Namesilo: API request to '$endpoint' failed with exit code $rc"

return "$rc"

fi

if [ -z "$response" ]; then

write_log 3 "Namesilo: Empty response from Namesilo API for '$endpoint'"

return 1

fi

printf '%s\n' "$response"

Copilot · 2025-12-26T09:19:42Z

net/ddns-scripts/files/usr/lib/ddns/update_namesilo_com_v1.sh

+}
+
+# update subdomain record
+rrid=$(call_api dnsListRecords "domain=$domain" | get_rrid)


The script fails to handle the case when no matching DNS record is found (empty rrid). If get_rrid returns an empty string, the dnsUpdateRecord API call will be made with an empty rrid parameter, which will result in an API error. The script should check if rrid is empty and either create a new record or report an error using write_log.

Suggested change

rrid=$(call_api dnsListRecords "domain=$domain" | get_rrid)

rrid=$(call_api dnsListRecords "domain=$domain" | get_rrid)

if [ -z "$rrid" ]; then

write_log 14 "No matching DNS record found for host '$username' in domain '$domain'"

exit 1

fi

Copilot · 2025-12-26T09:19:43Z

net/ddns-scripts/files/usr/lib/ddns/update_namesilo_com_v1.sh

+# wrap some routines
+call_api() {
+	wget -q -O- "https://www.namesilo.com/api/$1?version=1&type=xml&key=$password&$2"


The script depends on 'wget-ssl' but does not check if wget is available or has SSL support. Following the pattern of other scripts like update_godaddy_com_v1.sh (which checks for CURL and CURL_SSL), this script should verify wget availability and SSL support early in execution before attempting to use it.

Suggested change

# wrap some routines

call_api() {

wget -q -O- "https://www.namesilo.com/api/$1?version=1&type=xml&key=$password&$2"

# verify that wget with SSL/HTTPS support is available

: "${WGET_BIN:=wget}"

if ! command -v "$WGET_BIN" >/dev/null 2>&1; then

echo "DDNS: wget not found but is required for Namesilo updates" >&2

exit 1

fi

if ! "$WGET_BIN" --version 2>/dev/null | grep -qi 'https'; then

echo "DDNS: installed wget lacks SSL/HTTPS support, wget-ssl is required for Namesilo updates" >&2

exit 1

fi

# wrap some routines

call_api() {

"$WGET_BIN" -q -O- "https://www.namesilo.com/api/$1?version=1&type=xml&key=$password&$2"

new service provider namesilo.com, rewrite follow review guides. config: * set [domain] to apex domain * set [username] to subdomain (without apex domain) * set [password] to api key optional parameters: * ttl - ttl in seconds (ex: ttl=7200, default 3600) * pp - proxy protocol (ex: pp=socks5h, default http) Signed-off-by: Lin Fan <im.linfan@gmail.com>

GeorgeSapkin requested changes Dec 16, 2025

View reviewed changes

GeorgeSapkin added enhancement not following guidelines needs work labels Dec 16, 2025

cdfmr force-pushed the ddns-namesilo branch from 746c697 to a45ff15 Compare December 17, 2025 08:37

cdfmr requested a review from GeorgeSapkin December 17, 2025 08:42

cdfmr force-pushed the ddns-namesilo branch 3 times, most recently from 530a7ff to b142d07 Compare December 19, 2025 01:28

GeorgeSapkin requested changes Dec 19, 2025

View reviewed changes

GeorgeSapkin removed the not following guidelines label Dec 19, 2025

ddns-scripts: add namesilo.com

f7b88f2

new service provider namesilo.com config guide: * set [domain] to apex domain * set [username] to subdomain (without apex domain) * set [password] to api key Signed-off-by: Lin Fan <im.linfan@gmail.com>

cdfmr force-pushed the ddns-namesilo branch from b142d07 to f7b88f2 Compare December 19, 2025 01:54

cdfmr requested a review from GeorgeSapkin December 19, 2025 14:08

Merge branch 'master' into ddns-namesilo

566e206

feckert requested a review from Copilot December 25, 2025 18:04

Copilot AI reviewed Dec 25, 2025

View reviewed changes

Copilot started reviewing on behalf of feckert December 25, 2025 18:28 View session

BKPepe requested a review from Copilot December 26, 2025 09:14

Copilot started reviewing on behalf of BKPepe December 26, 2025 09:15 View session

Copilot AI reviewed Dec 26, 2025

View reviewed changes

Merge branch 'openwrt:master' into ddns-namesilo

e9418be

cdfmr force-pushed the ddns-namesilo branch from 94d4956 to 7c7e818 Compare January 7, 2026 01:05

cdfmr force-pushed the ddns-namesilo branch from 7c7e818 to 480a088 Compare January 7, 2026 01:08

		@@ -0,0 +1,28 @@
		#
		#.Distributed under the terms of the GNU General Public License (GPL) version 2.0

	rrid=`call_api dnsListRecords "domain=$domain" \| get_rrid`
	rrid=$(call_api dnsListRecords "domain=$domain" \| get_rrid)

-call_api() {
+call_api() {
+	# SECURITY NOTE:
+	# The NameSilo API requires the API key to be passed as a URL query parameter.
+	# This means the key may be exposed in shell history, process listings (ps),
+	# and system logs that record full command lines. Use a dedicated API key and
+	# protect access to this system and its logs accordingly.

	__TTL=3600
	# TTL (seconds); can be overridden by param_opt_ttl if provided by ddns config
	__TTL="${param_opt_ttl:-3600}"

-call_api dnsUpdateRecord "domain=$domain&rrid=$rrid&rrhost=$username&rrvalue=$__IP&rrttl=$__TTL" | grep success
+if call_api dnsUpdateRecord "domain=$domain&rrid=$rrid&rrhost=$username&rrvalue=$__IP&rrttl=$__TTL" | grep -q success; then
+	write_log 7 "Namesilo: DNS record for '$username.$domain' updated successfully to IP '$__IP'"
+else
+	write_log 3 "Namesilo: Failed to update DNS record for '$username.$domain' to IP '$__IP'"
+	return 1
+fi

		rrid=$(call_api dnsListRecords "domain=$domain" \| get_rrid)
		call_api dnsUpdateRecord "domain=$domain&rrid=$rrid&rrhost=$username&rrvalue=$__IP&rrttl=$__TTL" \| grep success

+[ -z "$username" ] && write_log 14 "ERROR: 'username' (sub domain) not set" && return 1
+[ -z "$password" ] && write_log 14 "ERROR: 'password' (api key) not set" && return 1
+[ -z "$domain" ] && write_log 14 "ERROR: 'domain' not set" && return 1

-	wget -q -O- "https://www.namesilo.com/api/$1?version=1&type=xml&key=$password&$2"
+	local endpoint="$1"
+	local params="$2"
+	local url="https://www.namesilo.com/api/${endpoint}?version=1&type=xml&key=${password}&${params}"
+	local response
+	response="$(wget -q -O- "$url")"
+	local rc=$?
+	if [ "$rc" -ne 0 ]; then
+		write_log 3 "Namesilo: API request to '$endpoint' failed with exit code $rc"
+		return "$rc"
+	fi
+	if [ -z "$response" ]; then
+		write_log 3 "Namesilo: Empty response from Namesilo API for '$endpoint'"
+		return 1
+	fi
+	printf '%s\n' "$response"

-# wrap some routines
-call_api() {
-	wget -q -O- "https://www.namesilo.com/api/$1?version=1&type=xml&key=$password&$2"
+# verify that wget with SSL/HTTPS support is available
+: "${WGET_BIN:=wget}"
+if ! command -v "$WGET_BIN" >/dev/null 2>&1; then
+	echo "DDNS: wget not found but is required for Namesilo updates" >&2
+	exit 1
+fi
+if ! "$WGET_BIN" --version 2>/dev/null | grep -qi 'https'; then
+	echo "DDNS: installed wget lacks SSL/HTTPS support, wget-ssl is required for Namesilo updates" >&2
+	exit 1
+fi
+# wrap some routines
+call_api() {
+	"$WGET_BIN" -q -O- "https://www.namesilo.com/api/$1?version=1&type=xml&key=$password&$2"

ddns-scripts: add namesilo.com #28095

Are you sure you want to change the base?

ddns-scripts: add namesilo.com #28095

Conversation

cdfmr commented Dec 16, 2025

📦 Package Details

🧪 Run Testing Details

✅ Formalities

Uh oh!

GeorgeSapkin commented Dec 16, 2025

Uh oh!

GeorgeSapkin left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cdfmr commented Dec 17, 2025

Uh oh!

GeorgeSapkin commented Dec 17, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Dec 26, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 26, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 26, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 26, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 26, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 26, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 26, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 26, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 26, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 26, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants