todotxt / todo.txt-cli

☑️ A simple and extensible shell script for managing your todo.txt file.
http://todotxt.org
GNU General Public License v3.0
5.56k stars 713 forks source link

Fail on broken symlink to avoid potential data loss #359

Open owenh000 opened 3 years ago

owenh000 commented 3 years ago

When the user attempts to run an add-on action that does not exist, todo.txt-cli prints a usage message and fails. However, todo.txt-cli supports add-ons that replace built-in actions. In this case, it is important to fail if there is something wrong with the add-on. Otherwise the user thinks they are still running the add-on while in reality they are only running the built-in action.

Here is an example scenario. This is based on real cases; see the again add-on README and the dorecur add-on README (of which I am the author). Other add-ons may be affected also.

  1. The user installs an add-on for handling recurring tasks. The add-on hooks into (overrides) the built-in do action, calling command do to access the built-in action as required. This is necessary to prevent the user from accidentally running do and discarding a recurring task.
  2. To facilitate updates, and because of limitations in how add-ons can be installed (see #214, for example), the add-on is installed as a local Git repository clone with a symlink in ~/.todo.actions.d/.
  3. At some point in the future, this Git repository is moved or the file is renamed, breaking the symlink.
  4. Instead of failing with an error, todo.txt-cli ignores the now-broken symlink, running the built-in action instead. Now recurring actions are being discarded (not replaced) without the user being aware of it.

Do you want to request a feature or report a bug?

That's open to interpretation, but I'll call this a bug.

What is the current behavior?

todo.txt-cli silently ignores an add-on that is a broken symbolic link.

If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem.

$ cd ~/.todo.actions.d
$ ln` -s ~/repos/todo.txt-cli-dorecur/dorecur.py do
$ todo add "Pay rent t:2021-09-28 due:2021-10-02 rec:+1m
TODO: 1 added.
$ todo do 1
1 (A) Pay rent t:2021-10-28 due:2021-11-02 rec:+1m
TODO: 1 added.
## Time passes and the user moves their Git repositories
$ mv ~/repos ~/git-repos
$ todo do 1
## todo.txt-cli silently fails to run the add-on and the recurring task is not replaced.

What is the expected behavior?

If:

then todo.txt-cli should fail with an error.

It seems reasonable to also apply this to other cases where an add-on is apparently installed but cannot be used, for example when an add-on file or directory lacks whatever read/execute permissions may be required.

Which versions todo.sh are you using?

$ todo-txt -V
TODO.TXT Command Line Interface v2.11.0

Which Operating System are you using?

Debian GNU/Linux Bullseye

Which version of bash are you using?

$ bash --version
GNU bash, version 5.1.4(1)-release (x86_64-pc-linux-gnu)
inkarkat commented 3 years ago

That's indeed a potential problem, though it would only affect power users with customizations that extend the built-in commands. I'm using such symlinks myself, and I also have done such reorganizations that required the update of them. If we don't limit this check to built-in commands, a small additional logic can detect and deal with it, as in my PR.

owenh000 commented 3 years ago

@inkarkat, thanks!

If I understand correctly, applying the check to all commands simply results in a more explicit error (versus a general failure with usage output) in the case of a non-built-in custom add-on. Thus the way you did it seems optimal to me, in addition to reducing required logic.

Also I tested your changes (e1c1c328a21e4c077688b4a50dbdc622f84d587e) and it looks good to me!

inkarkat commented 2 years ago

@owenh000 thanks for testing my changes!

Yes, you're right, in case of a custom add-on that's not overriding a built-in command, it now also complains about the broken symlink instead of assuming such an add-on command does not exist, and just printing the generic usage help.