Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HTML escaping of callback arguments #564

Closed
DrunkOnHotCoco opened this issue Aug 17, 2024 · 2 comments · Fixed by #565
Closed

HTML escaping of callback arguments #564

DrunkOnHotCoco opened this issue Aug 17, 2024 · 2 comments · Fixed by #565

Comments

@DrunkOnHotCoco
Copy link

DrunkOnHotCoco commented Aug 17, 2024

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
Copy link
Owner

Nukesor commented Aug 17, 2024

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
Copy link
Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants