ddns-scripts: allow for commands in $ip_script

[jenszo]

Maintainer: me / @jenszo Compile tested: n.a. (Runtime package script only) Run tested: mvebu/cortexa9, Linksys WRT3200ACM, OpenWrt 23.05.5 r24106-10cc5fcd00 / LuCI openwrt-23.05 branch git-24.264.56413-c7a3562, manually tested in place

Description:

The DDNS package provides a "script" method to detect the current IP that is to be reported to the DDNS provider. Because eventually it'll just be an eval $ip_script, this could likewise be a command instead.

A more notable example would be: dig -4 +short myip.opendns.com @resolver1.opendns.com

As a bonus, the user does not need to take care of backing up the script file before performing a sysupgrade with this being stored in /etc/config

Having this option is a much more convenient alternative to the "url" method. Effort-wise, however, a dedicated mode isn't really justified.

[xxxxxliil]

Can you try to compress the two commits into one? try: $ git commit --amend

[jenszo]

Since this my my first OPR to openWRT, I've read up on your versioning and release scheme here: https://www.reddit.com/r/openwrt/comments/lolxs2/a_guide_to_the_meaning_behind_openwrt_version/

So this PR is towards master. What would be the chances for such a smaller change to be incorporated into the next 23.05.06 patch release? If there are, how can I apply for this to happen?

I'm trying to figure out whether I should set up my own package for it while waiting for the next major release branch.

[jenszo]

Hopefully, this now resolves the formalities test failure on my name.

[feckert]

Formally, the code looks good. Personally, I don't like writing such commands into a uci config option. I think you have to consider a lot of things for escap sequennces for backtiggs I'm afraid that you can do a lot of things wrong when creating them.

[xxxxxliil]

Try encoding the command with base64/85/92? Or save the command to a file.

[systemcrash]

Formally, the code looks good. Personally, I don't like writing such commands into a uci config option. I think you have to consider a lot of things for escap sequennces for backtiggs I'm afraid that you can do a lot of things wrong when creating them.

Valid concern, but the field already handles scripts - so the danger is already present. Let's just give it more sharp edges :D

[jenszo]

Apologies for the delayed response.

There's definitely two different concerns here:

Indeed, from a security and integrity perspective, this isn't ideal to start with. Generally.

However, as @systemcrash has already pointed out, the original implementation is based on eval $ip_script with hardly any safeguards. Blindly running user-provided, executable code is always a gamble, regardless of the format. However, it is explicitly labelled as such. So responsibility is with the informed user, I guess.

As this is merely making use of what is already out there I'd argue to either pull it completely or allow this shortcut for convenience.

Aside from the security aspect, I definitely should look into storing it safely and consistently to not conflict with the config format resulting in broken or partial command execution. Thanks for mentioning it @feckert, this totally slipped my mind.

Edit: The way I read the uci docs, we need to take care of single and double quotes