on_error and Apprise causing Borgmatic to fail backups #839

New Issue

ericswpark · 2024-03-08T20:43:45Z

ericswpark commented

2024-03-08 20:43:45 +00:00

What I'm trying to do and why

I'm trying to figure out why my Borgmatic + Apprise configuration stopped working from a Borgmatic update, when it was working fine before. The problematic config lines are the following:

on_error:
  - apprise --title="Borgmatic" --body="An error occurred.\n{error}\n{output}" -e

Steps to reproduce

Try and run borgmatic with the configuration that includes the lines provided above

Actual behavior

It produces the following error:

/etc/borgmatic.d/config.yaml: Error running on-error hook
Usage: apprise [OPTIONS] SERVER_URL [SERVER_URL2 [SERVER_URL3]]
Try 'apprise -h' for help.
Error: No such option: --keep-daily
Command 'apprise --title="Borgmatic" --body="An error occurred.\n'Command '"'"'borg prune --keep-daily 7 --keep-hourly 24 --keep-monthly 6 --keep-yearly 5 --glob-archives server-1-* --stats --info --list ssh://borg@server-2/./server-1.borg'"'"' returned non-zero exit status 2.'\n'...
  File "/usr/lib/python3/dist-packages/borg/remote.py", line 368, in open
(...)

It seems like the {error} variable does some quote escaping, which manages to escape out of the --body="" quotes.

Expected behavior

Borgmatic should not fail to run with this configuration, by doing the quote escaping properly or not at all.

Other notes / implementation ideas

Transferred from https://github.com/borgmatic-collective/docker-borgmatic/issues/306

borgmatic version

1.8.8

borgmatic installation method

Docker container

Borg version

1.2.7

Python version

3.11.5

Database version (if applicable)

N/A

Operating system and version

Slackware 15.0 (unRAID)

### What I'm trying to do and why I'm trying to figure out why my Borgmatic + Apprise configuration stopped working from a Borgmatic update, when it was working fine before. The problematic config lines are the following: ``` on_error: - apprise --title="Borgmatic" --body="An error occurred.\n{error}\n{output}" -e ``` ### Steps to reproduce Try and run borgmatic with the configuration that includes the lines provided above ### Actual behavior It produces the following error: ``` /etc/borgmatic.d/config.yaml: Error running on-error hook Usage: apprise [OPTIONS] SERVER_URL [SERVER_URL2 [SERVER_URL3]] Try 'apprise -h' for help. Error: No such option: --keep-daily Command 'apprise --title="Borgmatic" --body="An error occurred.\n'Command '"'"'borg prune --keep-daily 7 --keep-hourly 24 --keep-monthly 6 --keep-yearly 5 --glob-archives server-1-* --stats --info --list ssh://borg@server-2/./server-1.borg'"'"' returned non-zero exit status 2.'\n'... File "/usr/lib/python3/dist-packages/borg/remote.py", line 368, in open (...) ``` It seems like the `{error}` variable does some quote escaping, which manages to escape out of the `--body=""` quotes. ### Expected behavior Borgmatic should not fail to run with this configuration, by doing the quote escaping properly or not at all. ### Other notes / implementation ideas Transferred from https://github.com/borgmatic-collective/docker-borgmatic/issues/306 ### borgmatic version 1.8.8 ### borgmatic installation method Docker container ### Borg version 1.2.7 ### Python version 3.11.5 ### Database version (if applicable) N/A ### Operating system and version Slackware 15.0 (unRAID)

witten commented

2024-03-08 21:36:14 +00:00

Thanks for (re)filing this! Since borgmatic #810 in 1.8.7, interpolated hook variables are escaped to prevent shell injection attacks, and so you are correct that escaping of the {error} value is what's causing problems here. Specifically, the problem looks to be that {error} is escaped as if it was a single argument when in fact it's contained in quotes within your --body value, ironically resulting in the "break-out" that causes the Apprise error you see.

Unfortunately I'm not sure of a good way around this at the escaping level. That's because the escaping function I'm using (shlex.escape()) isn't smart enough to know about the context in which it's used. I'm open to suggestions though.

However, I do have an idea for an alternative. If borgmatic's existing Apprise hook included support for sending logs (and therefore errors) to Apprise notification services, would that address your need here? I'm thinking of adding an optional send_logs flag that could be used as follows:

apprise:
    services:
        - url: gotify://hostname/token
          label: gotify
    send_logs: true
    ...
    fail:
        title: Uh oh!
        body: Your backups have failed. See logs for details.

That would cause full borgmatic logs to be added automatically to the Apprise body upon send.

Anyway, let me know your thoughts!

EDIT: Moved send_logs: up a level, because I don't think it needs to be configurable per state.

Thanks for (re)filing this! Since borgmatic #810 in 1.8.7, interpolated hook variables are escaped to prevent shell injection attacks, and so you are correct that escaping of the `{error}` value is what's causing problems here. Specifically, the problem looks to be that `{error}` is escaped as if it was a single argument when in fact it's contained in quotes within your `--body` value, ironically resulting in the "break-out" that causes the Apprise error you see. Unfortunately I'm not sure of a good way around this at the escaping level. That's because the escaping function I'm using (`shlex.escape()`) isn't smart enough to know about the context in which it's used. I'm open to suggestions though. However, I do have an idea for an alternative. If borgmatic's existing [Apprise hook](https://torsion.org/borgmatic/docs/how-to/monitor-your-backups/#apprise-hook) included support for sending logs (and therefore errors) to Apprise notification services, would that address your need here? I'm thinking of adding an optional `send_logs` flag that could be used as follows: ```yaml apprise: services: - url: gotify://hostname/token label: gotify send_logs: true ... fail: title: Uh oh! body: Your backups have failed. See logs for details. ``` That would cause full borgmatic logs to be added automatically to the Apprise `body` upon send. Anyway, let me know your thoughts! EDIT: Moved `send_logs:` up a level, because I don't think it needs to be configurable per state.

ericswpark commented

2024-03-08 21:46:32 +00:00

Yup, the send_logs flag would definitely be very useful! The main reason I want the log included in the notifying email is because it's a bit of a chore to dig through the Docker logs to figure out where the error is, especially if Borgmatic has since done more automated backups following the schedule.

Yup, the `send_logs` flag would definitely be very useful! The main reason I want the log included in the notifying email is because it's a bit of a chore to dig through the Docker logs to figure out where the error is, especially if Borgmatic has since done more automated backups following the schedule.

witten commented

2024-03-08 21:56:33 +00:00

Okay, thanks. I'll work on getting send_logs supported. (It would still probably be a good idea to fix the escaping, but right now I don't have line of sight to a solution for that part.)

Okay, thanks. I'll work on getting `send_logs` supported. (It would still probably be a good idea to fix the escaping, but right now I don't have line of sight to a solution for that part.)

❤️ 1

witten commented

2024-03-10 23:31:53 +00:00

send_logs is now implemented in main, enabled by default without having to configure anything additional, and will be part of the next release! I also added a logs_size_limit option under apprise: which may be of use.

Documentation is here: https://torsion.org/borgmatic/docs/how-to/monitor-your-backups/#apprise-hook

Feedback welcome.

I'm going to leave this ticket open a bit longer though to look at the original escaping issue when I get a chance.

`send_logs` is now implemented in main, enabled by default without having to configure anything additional, and will be part of the next release! I also added a `logs_size_limit` option under `apprise:` which may be of use. Documentation is here: https://torsion.org/borgmatic/docs/how-to/monitor-your-backups/#apprise-hook Feedback welcome. I'm going to leave this ticket open a bit longer though to look at the original escaping issue when I get a chance.

🚀 2

witten referenced this issue from a commit

2024-03-11 17:43:10 +00:00

Document a potentially breaking shell quoting edge case within error hooks (#839).

witten commented

2024-03-11 17:57:22 +00:00

Well, I ended up "fixing" the escaping issue by documenting around it. (See the bottom of that section.)

Hopefully though it's no longer as much of an issue now that the Apprise hook can do its own logging.

Calling this done for now.

Well, I ended up "fixing" the escaping issue by [documenting around it](https://torsion.org/borgmatic/docs/how-to/monitor-your-backups/#error-hooks). (See the bottom of that section.) Hopefully though it's no longer as much of an issue now that the Apprise hook can do its own logging. Calling this done for now.

witten closed this issue

2024-03-11 17:57:22 +00:00

witten commented

2024-03-11 20:30:30 +00:00

Released in borgmatic 1.8.9!