Use Astral-maintained tokio-tar fork

[charliermarsh]

Summary

I shipped one security fix here along with several significant performance improvements for large TAR files:

• Avoid creating directories outside of target tokio-tar#2
• Buffer writes to archive entries tokio-tar#4
• Memoize filesystem operations during unpack tokio-tar#5

I also PR'd the security fix to edera-dev (edera-dev/tokio-tar#4).

[charliermarsh]

Converting to draft while I checkout failing debug assertions.

[charliermarsh]

This is related to astral-sh/tokio-tar#2. I forgot that we don't actually use the crate's unpack method, we unpack ourselves (since we need to set permissions in a different way).

[charliermarsh]

We can get rid of this whole vendored unpacked_at implementation. It turns out the crate now returns the target path. (That's not a change I made, it must've been made in one of the forks.)

[konstin]

Is this a concern for us? We usually create directories from archives without preserving permissions (at least in the zip path we should).

[Gankra]

Also maybe this is me being unfamiliar with the codebase but this is so... bizarre. How does this unpacking a file before the dir that contains it make sense? Is the idea that we create_dir_all any file path anyway, so the dir entries only create empty dirs and set perms on dirs that have files?

[konstin]

For zips we always create the parent of a file, as a zip can contain directory entries, but sometimes there are no directory entries, just files.

uv/crates/uv-extract/src/stream.rs

Lines 66 to 76 in 4d3809c

	// Either create the directory or write the file to disk.
	if is_dir {
	if directories.insert(path.clone()) {
	fs_err::tokio::create_dir_all(path).await?;
	}
	} else {
	if let Some(parent) = path.parent() {
	if directories.insert(parent.to_path_buf()) {
	fs_err::tokio::create_dir_all(parent).await?;
	}
	}

[charliermarsh]

Is the idea that we create_dir_all any file path anyway, so the dir entries only create empty dirs and set perms on dirs that have files?

Yes. When we create a file, we create the directories required for it. This happens within the async-tar crate. The reason we have to do it "manually" here is because we have our own unpack that replicates the async-tar logic but applies different permissions.

[charliermarsh]

(I will look at why the tar crate introduced this before merging.)

[charliermarsh]

It kind of seems like a bug that we aren't preserving directory permissions in the zip case?

[konstin]

personally, i'd prefer only preserving the executable bit and otherwise using the default umask; i'd find it odd if the permissions on the build host should determine the permissions in the deployment target.

[charliermarsh]

Yeah. Probably requires more invasive changes to the tar crate though.

[charliermarsh]

I'd like to review what pip does here before making other changes (since this is just preserving our status quo).

[charliermarsh]

Nevermind, pip explicitly only preserves the executable bit. I'll make the same change for tar in a separate PR.

https://github.com/pypa/pip/blob/3898741e29b7279e7bffe044ecfbe20f6a438b1e/src/pip/_internal/utils/unpacking.py#L88-L100