search

junegunn / fzf

:cherry_blossom: A command-line fuzzy finder
https://junegunn.github.io/fzf/
MIT License
62.32k stars 2.35k forks source link

Tarball extraction in install.sh — improvement suggestion #3776

Closed rseichter closed 2 months ago

rseichter commented 2 months ago

Checklist

Output of fzf --version

0.51.0 (260a65b)

OS

Shell

Problem / Steps to reproduce

In order to avoid ownership and attribute issues when running the installation shell script as root, I'd like to suggest using additional options, like so:

tar --no-xattrs --no-same-owner -xzf foobar.tar.gz

I am aware that not every implementation of tar supports these options (GNU Tar does), so the script would need to smartly and dynamically fall back to a simpler call to tar. As an alternative, install.sh might be made to accept an environment variable like TAR_EXTRACT_CMD, or script arguments like --tar-extract-cmd. This would allow users maximum flexibility.

GNU Tar is quite common, and when run as root, it tries to restore the original files' owner, group and full attributes. That is not something I want to happen on my machines. Currently, I need a post-installation step to correct this, and I would rather Fzf install.sh took care of it.

What do you think?

junegunn commented 2 months ago

Currently, I need a post-installation step to correct this

Can you show the exact steps, and how things look before and after the steps?

What do you think?

Maybe. We can use the options if

  1. either when gtar is available on then system
  2. or when $TAR | grep -qF no-same-owner succeeds

FWIW, these days I don't think install script is that important, because most users just use a package manager (brew on macOS), and because the binary now embeds the shell integration scripts (but not fzf-tmux though).

rseichter commented 2 months ago

My post-installation steps consist of forcibly changing attributes via Ansible. I don't have the playbook at hand right now, but here is what things look like immediately after a fresh Fzf installation:

❯ ls -ln /usr/local/bin
total 4.7M
-rwxr-xr-x 1 501 50 3.5M May  1 05:36 fzf*
-rwxr-xr-x 1   0  0 1.2M May  6 18:57 zoxide*

Non-root-owned executables in /usr/local/bin obviously pose a security risk. In bash, the step to fix things would be simply chown root:root /usr/local/bin/fzf.

As for the importance of install.sh versus package managers, I'll just say it depends. Multi-platform provisioning using tools like Ansible has its perks, and package names vary across platforms, while /usr/local/bin can usually be relied on.

Regarding the implementation, I guess this would work:

tar --no-same-owner --no-xattrs -xf foo.tar || tar -xf foo.tar

junegunn commented 2 months ago

Regarding the implementation, I guess this would work:

tar --no-same-owner --no-xattrs -xf foo.tar || tar -xf foo.tar

We're currently using tar with the standard input pipe (curl -fL $1 | tar -xzf -), so I don't think || trick will do unless we change it to save to a temporary file.

junegunn commented 2 months ago

Looks like tar of macOS Sonoma understands --no-same-owner and --no-xattrs, so we can just add it?

By the way, why do we need --no-xattrs?

rseichter commented 2 months ago

Explicitly disabling support for extended file attributes is merely another precaution against possible surprises. GNU Tar behaves like this by default, but I am uncertain about other platforms.

I guess that considering the peculiarities of different tar implementations goes to show how old I have become. These things really mattered, back in the day. 😉

In any case, thank you for the update, that was quick indeed.

junegunn commented 2 months ago

No problem. Thanks for the explanation. Let's see if anyone complains.