Add support of absolute paths

[zxvfc]

Add preserve_absolute to builder's options to let it add files with absolute path.

By default preserve_absolute sets false, to keep current behavior, but it can be set as true via passing true to builder's method preserve_absolute.

Closes: #326, #413

[anonhostpi]

Solid simple fix!

For testing, do you think it would be worth it to generate a tar with an absolute path in it using an external tar packer (probably GNU tar, since they are explicit with their absolute path support), unpack it with this crate, then verify the file actually exists?

Would require spawn/cli calls from the test process, but would be comprehensive.

[zxvfc]

@anonhostpi, thanks! Hm, yes, test unpack of GNU tar created archive sounds valid. I'll try to implement it.

[Pathal]

Another solution would be to leverage crate features.

Adding to the header.rs::copy_path_into_inner function's match, you don't have to cascade the flag through all that code and make people explicitly opt into "less safe" tar-balls at the dependency level.

#[cfg(not(feature = "absolute-paths"))]
(Component::Prefix(..), false) | (Component::RootDir, false) => return Err(other("paths in archives must be relative"));

and

#[cfg(feature = "absolute-paths")]
(_, false) => {}

I'm also working on a k8s project where leveraging tar-rs with absolute paths would come in handy to (ironically enough) improve pipeline security.

[zxvfc]

Hi @anonhostpi! What do you think? Actually @Pathal suggested pretty clean solution (probably even cleaner then mine, since there is no need to pass around allow_absolute flag). Does it make sense to refactor my implementation to that?

[anonhostpi]

Hi @anonhostpi! What do you think?

Appreciate the added test. It will ensure that the library is at the same competency as other projects.

Actually @Pathal suggested pretty clean solution (probably even cleaner then mine, since there is no need to pass around allow_absolute flag). Does it make sense to refactor my implementation to that?

Yes and no. Using feature flags offloads tech debt and forces the developer to decide on opt-in, not the end-user.

I think @Pathal's solution is a good one, but it needs to be documented, that if someone uses tar-rs to develop a rust-based tar CLI (or other tar client) competitive with GNU tar:

• not only do they have to enable the feature-flag, but...
• should implement a feature-gate (much like your initial implementation) in their code to prevent insecure packing and unpacking after enabling the feature-flag

In short, I think it is good, but we should be clear on why we decided that, and what debt is offloaded to the end-developer.

[anonhostpi]

I'm also working on a k8s project where leveraging tar-rs with absolute paths would come in handy to (ironically enough) improve pipeline security.

Honestly, IMO tar absolute paths themselves aren't a security risk. It's the poor understanding of them that is. Absolute paths in tarballs are phenomenal for snapshots and patching, making them invaluable to security maintenance.

They're really good for rollback functionality. I believe I've seen tar absolute paths used extensively in systems that offer "factory resets" to default settings/builds by destroying an overlayfs and rebuilding it with one of these tarballs.

[Pathal]

Agreed on all accounts. I'm fine with this as is (not that I have any say in the matter 😂).

[anonhostpi]

Afternoon @alexcrichton,

Request to merge #426

Let me know if you need anything. Tried looking for a patreon or funding page on your profile to buy you a coffee or a donut, but didn't find one. Let me know if you want one.

Thanks

Hello! And apologies for the delay. I've been hoping to mostly hand off maintenance of tar-rs to @cgwalters or @xzfc on the repo, would you be ok trying to reach out to them to ask for review?

I realize the maintenance story for tar-rs isn't in the best spot right now, cgwalters/xzfc don't seem too too active and may have moved on (unsure). I unfortunately don't have a ton of time/motivation right now to maintain the crate :(

@cgwalters and @xzfc have been named as the maintainers. I'll try to poke them for code review.

[cgwalters]

I said this elsewhere but what I think we want at some point is to split out a "tar-core" crate that's "sans io" and allows constructing nearly arbitrary tar streams (but of course still giving high level sugar by default to handle e.g. GNU long links and such).

Anyways I think the biggest blocker for this PR as is is that it's a semver break in changing the signature of at least pub fn append_data and I think that would require a lot of nontrivial ecosystem churn for a relatively niche use case.

I think we just need the global option, there's no need to allow it per-data addition?

[cgwalters]

it's a semver break

Also we should add cargo-semver-checks here

[zxvfc]

Thanks @cgwalters for your comments. Yes, you are right, it is really breaking semantic, which is not very good.

I'll see how to rework it and come back with a new changes. :)

[anonhostpi]

niche

Niche is a strong word. It seems niche, because a lot of tar users don't understand that it exists.

I have seen absolute path usage used extensively in security tools and package management software. As far as I know, absolute pathing in tar has been a part of its standard abilities since its initial release with v7 Unix at Bell labs.

The way I've seen it primarily used is for restoring and/or ensuring dependent paths when administering packages and/or performing secure "factory resets"

Secure "factory resetting" is a huge use case for the feature.

[zxvfc]

Hello @anonhostpi, @cgwalters!

As promised, I'm back with some rework.
I've removed the allow_absolute parameter from Builder's public methods (append_data and append_writer). These methods now use the internal self.options.preserve_absolute setting instead, which avoids the semver break you mentioned.

However the Header::set_path() and Header::set_link_name() methods still have the allow_absolute parameter added. I kept this because: Header seems pretty low level; The Header structure doesn't have an options and I'm not sure that it's good to change it.

Anyway, let me know what do you think about it.
Thanks!