junegunn/fzf

Tarball extraction in install.sh — improvement suggestion

rseichter opened this issue · 6 comments

rseichter commented

Checklist

  • I have read through the manual page (man fzf)
  • I have searched through the existing issues
  • For bug reports, I have checked if the bug is reproducible in the latest version of fzf

Output of fzf --version

0.51.0 (260a65b)

OS

  • Linux
  • macOS
  • Windows
  • Etc.

Shell

  • bash
  • zsh
  • fish

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

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

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

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

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

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

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