Support user directory expansion in credential hooks

Alexander-Shukaev commented

2021-05-30 20:09:01 +00:00

Apart from source_directories, none of the configuration options seem to support tilde (~). That is such paths are passed through as is to borg or otherwise acted upon by borgmatic without applying os.path.expanduser. Would be nice to have this feature applied to paths coming from configuration file in general. Perhaps normalizing (sanitizing) paths is also a good idea.

Apart from `source_directories`, none of the configuration options seem to support tilde (`~`). That is such paths are passed through as is to `borg` or otherwise acted upon by `borgmatic` without applying `os.path.expanduser`. Would be nice to have this feature applied to paths coming from configuration file in general. Perhaps normalizing (sanitizing) paths is also a good idea.

👍 1

witten commented

2021-06-07 17:52:55 +00:00

Owner

This seems reasonable to me, but I'm curious: Are there specific options where you'd like to see expansion applied? Candidates I'm seeing:

patterns_from
excludes_from
borgmatic_source_directory
temporary_directory
borg_*_directory
repositories and check_repositories: Already expands ~ implicitly for remote repositories due to SSH being used. I suppose it'd be handy to expand for local repository paths too?
PostgreSQL ssl_* paths.

Ones I'd probably omit:

patterns: Would require annoying parsing, as it's not just plain paths. EDIT: Although maybe only "root" patterns could support this, as those are just plain paths?
encryption_passcommand: May require parsing so as not to expand command arguments, too.
hooks: Same reason.

Let me know your thoughts. Thanks!

This seems reasonable to me, but I'm curious: Are there specific options where you'd like to see expansion applied? Candidates I'm seeing: * `patterns_from` * `excludes_from` * `borgmatic_source_directory` * `temporary_directory` * `borg_*_directory` * `repositories` and `check_repositories`: Already expands `~` implicitly for remote repositories due to SSH being used. I suppose it'd be handy to expand for local repository paths too? * PostgreSQL `ssl_*` paths. Ones I'd probably omit: * `patterns`: Would require annoying parsing, as it's not just plain paths. EDIT: Although maybe only "root" patterns could support this, as those are just plain paths? * `encryption_passcommand`: May require parsing so as not to expand command arguments, too. * `hooks`: Same reason. Let me know your thoughts. Thanks!

ams_tschoening commented

2021-07-15 08:15:08 +00:00

encryption_passcommand: May require parsing so as not to expand command arguments, too.

Though, that's exactly the use case I'm interested in: I have a setup with one special user doing backups, maintaining all data necessary for borg in its home directory and I store passwords there as well. The entire home is stored on an encrypted ZFS, which gets mounted manually, so it's somewhat safe. Therefore, my passcommand looks like the following right now:

encryption_passcommand: cat /home/ams_d_bak_borg/.config/borg/passwds/hosts

While it would be a little easier to have the following instead:

encryption_passcommand: cat ~/.config/borg/passwds/hosts

Slightly less typing and things are better prepared against additional backup users or a renamed user or a relocated home directory or alike. cat is even used in the official borg-docs, so supporting ~ for its arguments makes sense to me.

> encryption_passcommand: May require parsing so as not to expand command arguments, too. Though, that's exactly the use case I'm interested in: I have a setup with one special user doing backups, maintaining all data necessary for borg in its home directory and I store passwords there as well. The entire home is stored on an encrypted ZFS, which gets mounted manually, so it's somewhat safe. Therefore, my passcommand looks like the following right now: encryption_passcommand: cat /home/ams_d_bak_borg/.config/borg/passwds/hosts While it would be a little easier to have the following instead: encryption_passcommand: cat ~/.config/borg/passwds/hosts Slightly less typing and things are better prepared against additional backup users or a renamed user or a relocated home directory or alike. `cat` is even used in the official borg-docs, so supporting `~` for its arguments makes sense to me.

witten commented

2021-07-16 15:49:30 +00:00

Owner

Thanks, that example is helpful. It sounds like it's actually the command arguments that you want expanded! So the way I could see this working is to split the value of encryption_passcommand (and other options that are passed to Borg via environment variables), expand each individual command/argument in case there are tildes, and then join them back together. I think that would achieve what you're looking for.

Thanks, that example is helpful. It sounds like it's actually the command arguments that you want expanded! So the way I could see this working is to split the value of `encryption_passcommand` (and other options that are passed to Borg via environment variables), expand each individual command/argument in case there are tildes, and then join them back together. I think that would achieve what you're looking for.

👍 1

witten added the

good first issue

label

2023-03-27 19:05:36 +00:00

120EE0980 commented

2025-03-22 02:42:19 +00:00

Contributor

I want to work on this issue.

witten commented

2025-03-22 04:37:26 +00:00

Owner

Great! A lot has changed since this ticket was originally filed, so here are some thoughts: Instead of trying to split and parse encryption_passcommand, it might make more sense to instead support directory expansion in these two types of crendential hooks:

My rationale is that those two hooks are really intended to replace the use case of cating a file with encryption_passcommand—and they have distinct file paths that would be easier to expand since they're already parsed.

As for other cases for expanding ~, I think that modern borgmatic patterns (and therefore source_directories, excludes, patterns_from, etc. which all roll into patterns) are all already expanded. You could verify that if you wanted to. You could also see if there are any other good candidate options in borgmatic for adding ~ expansion. But personally, I'd just try to keep this ticket relatively small and scoped rather than trying to add expansion absolutely everywhere.

Your call though how you want to approach this. This is just my off-the-cuff brainstorming.

Great! A lot has changed since this ticket was originally filed, so here are some thoughts: Instead of trying to split and parse `encryption_passcommand`, it might make more sense to instead support directory expansion in these two types of crendential hooks: * [file-based crendentials](https://torsion.org/borgmatic/docs/how-to/provide-your-passwords/#file-based-credentials) * [KeePassXC credentials](https://torsion.org/borgmatic/docs/how-to/provide-your-passwords/#keepassxc-passwords) My rationale is that those two hooks are really intended to replace the use case of `cat`ing a file with `encryption_passcommand`—and they have distinct file paths that would be easier to expand since they're already parsed. As for other cases for expanding `~`, I *think* that modern borgmatic `patterns` (and therefore `source_directories`, `excludes`, `patterns_from`, etc. which all roll into `patterns`) are all already expanded. You could verify that if you wanted to. You could also see if there are any other good candidate options in borgmatic for adding `~` expansion. But personally, I'd just try to keep this ticket relatively small and scoped rather than trying to add expansion absolutely everywhere. Your call though how you want to approach this. This is just my off-the-cuff brainstorming.

120EE0980 commented

2025-03-23 02:11:45 +00:00

Contributor

Thanks for the detailed info. I have started working on this issue.But I need some time before submitting a PR as I am working on the proposal.