From: Manuel Federanko <m.federanko@proxmox.com>
To: Wolfgang Bumiller <w.bumiller@proxmox.com>
Cc: pbs-devel@lists.proxmox.com
Subject: Re: [PATCH proxmox-backup v2] fix #5247: relative paths in exclude patterns.
Date: Tue, 31 Mar 2026 16:48:14 +0200 [thread overview]
Message-ID: <1fcc11ea-e66c-414e-a6f4-9ab43d0dae87@proxmox.com> (raw)
In-Reply-To: <177496589589.113663.12113566106144305854.b4-review@b4>
On 2026-03-31 4:04 PM, Wolfgang Bumiller wrote:
> On Mon, 30 Mar 2026 10:11:10 +0200, Manuel Federanko <m.federanko@proxmox.com> wrote:
>> Patterns which start with ./ or for that matter contain /../ or similar
>> constructs will never match, since they get compared to sanitized paths
>> from directory traversal, sanitize those patterns too. Implement this
>> for both .pxarexclude files and the --exclude command line option.
>> Log a warning if a path was sanitized that was provided via --exclude.
>
> In the `.pxarexclude` case, logging may make sense as well IMO.
ack
> As for `--exclude`, I wonder we should just *bail* instead?
>
> Although that might "break" some automated
> otherwise-working-but-probably-too-big backups, so we can't... (although
> for `foo/../bar` it's *really* tempting...)
I'm a bit undecided, maybe log a warning with a deprecation notice that
this will lead to an error in the future? Same goes for the cli option
now that I'm thinking about it.
>>
>>
>> diff --git a/pbs-client/src/pxar/tools.rs b/pbs-client/src/pxar/tools.rs
>> index 475c08ad3..7ad4009fe 100644
>> --- a/pbs-client/src/pxar/tools.rs
>> +++ b/pbs-client/src/pxar/tools.rs
>> @@ -76,6 +76,37 @@ fn assert_single_path_component_do(path: &Path) -> Result<(), Error> {
>> Ok(())
>> }
>>
>> +pub fn normalize_lexically<S: AsRef<OsStr> + ?Sized>(path: &S) -> PathBuf {
>
> (Note: the monomorphization from v1 was actually fine, but if the `_do`
> variant is only used by this one function it could live *inside* here.)
>
>> + // FIXME: Once std::path::normalize_lexically is stabilized we can
>> + // switch to that
>> + use std::path::Component;
>> +
>> + let path = Path::new(path);
>> + let has_trailing_slash = path
>> + .as_os_str()
>> + .as_encoded_bytes()
>> + .last()
>> + .copied()
>> + .is_some_and(|c: u8| c == std::path::MAIN_SEPARATOR_STR.bytes().last().unwrap());
>
> Could just use
>
> .as_encoded_bytes()
> .ends_with(MAIN_SEPARATOR_STR.as_bytes())
ack, will change in v3 once warning/bailing has been decided
prev parent reply other threads:[~2026-03-31 14:47 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-03-30 8:11 Manuel Federanko
2026-03-31 14:04 ` Wolfgang Bumiller
2026-03-31 14:48 ` Manuel Federanko [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1fcc11ea-e66c-414e-a6f4-9ab43d0dae87@proxmox.com \
--to=m.federanko@proxmox.com \
--cc=pbs-devel@lists.proxmox.com \
--cc=w.bumiller@proxmox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.