Nukesor / pueue

:stars: Manage your shell commands.
Apache License 2.0
5.03k stars 133 forks source link

HTML escaping of callback arguments #564

Closed DrunkOnHotCoco closed 3 months ago

DrunkOnHotCoco commented 3 months ago

It seems the existing callback templating will HTML-escape the output.

For example, the callback from the Wiki: callback: '/path/to/callback "{{id}}" "{{command}}" "{{path}}" "{{result}}" "{{group}}" "{{start}}" "{{end}}"' , for a command such as pueue add "sleep 1 && echo hi" will provide command as sleep 1 && echo hi to the callback script. This extends to a bunch of common characters like instances of a single quote ' becoming &#x27, and also to all the possible task fields you can provide to the callback, including output.

This escaping can be avoided by using triple braces rather than double braces for the templating.

While this behavior seems to be the default for Handlebars https://handlebarsjs.com/guide/expressions.html#html-escaping, I wasn't familiar and spent some time hunting things down. I think there should be a note in the Wiki in the callback section, perhaps something like:

Note that the above Handlebars templating with double braces {{ variable }} will HTML-escape the output. Avoid this by using triple braces {{{ variable }}}.

, particularly since it isn't mentioned what provides the templating (nor do I necessarily think it should be). Perhaps the triple braces should even be used in the examples, but maybe the other notification examples rely it.

Note that other places in the code will outright disable this escaping such as:

https://github.com/Nukesor/pueue/blame/afcd28dbb8789b89ed3e2023996f9c0830bb9554/pueue_lib/src/process_helper/mod.rs#L63-L65

I think giving users the option to automatically escape the output is fine though, so not advocating for this to be done for the callback processing. Opened the issue since a) not sure if random contributions to Wiki are okay and b) searchability/exposure for others that might have this issue.

Steps to reproduce

  1. Add callback to config file such as callback: 'echo "{{command}}" > ~/callback.txt'
  2. Run a command with HTML characters pueue add "sleep 1 && echo hi"
  3. Observe output in output.txt as sleep 1 && echo hi

Debug logs (if relevant)

No response

Operating system

RHEL7, WSL2

Pueue version

v3.4.1

Additional context

No response

Nukesor commented 3 months ago

I'm not gonna lie, its kind of funny that this has gone unoticed for years.

This is definitely a bug, there's no reason why there should be html escaping for callback templating.

DrunkOnHotCoco commented 3 months ago

Great, thanks for the quick reply/change - and of course for pueue in the first place :)