Packaging questions

[LecrisUT]

Hi, I am trying to package this project for Fedora. Besides the issue of cutting a release (#157), I have a more general question about the scope of this project.

Is it ok to say that bash-preexec.sh is designed to be able to run outside of the interactive mode as well? I am asking because for the Fedora packaging I want to add this to the /etc/profile.d, but for security concerns I want to gate it to only be sourced if it's in an interactive environment. If necessary this gating could be disabled by overwriting the file that tests for interactive mode in /etc/profile.d, but that would be an explicit command requiring sudo. What do you think about this approach?

@akinomyoga is it ok to ask for your opinion as well?

[akinomyoga]

I assume you mean the interactive mode/shell of Bash by "the interactive mode". (It should be noted that there is also "the interactive mode of bash-preexec", which is unrelated to the interactive mode of Bash. The term "interactive mode" used in the codebase of bash-preexec is the mode turned on on the precmd hook and turned off on the preexec hook.)

Is it ok to say that bash-preexec.sh is designed to be able to run outside of the interactive mode as well?

As far as I can tell, bash-preexec.sh doesn't target the non-interactive mode. The concept of the hooks preexec and precmd is specific to the interactive use of the shell. Also, even if bash-preexec.sh is sourced, I think nothing would happen because PROMPT_COMMAND (which will invoke the precmd hook) is not active in the non-interactive shell, and the preexec will also never be executed because it is only enabled after a call of the precmd hook. In addition, the DEBUG trap will be called for every command, which would become a large overhead, although the trap doesn't do anything actually. Therefore, I think bash-preexec.sh should not be loaded in the non-interactive shell.

Maybe we can have an explicit check for the interactive shell at the beginning of bash-preexec.sh.

I am asking because for the Fedora packaging I want to add this to the /etc/profile.d,

I personally don't think it is a good idea to put the loader in /etc/profile.d. That will forcibly load bash-preexec.sh in all the users in the system including root. (In Fedora, it would be technically possible to disable it by removing . /etc/bashrc from ~/.bashrc, but that will disable all the other basic initialization and the other /etc/profile.d scripts.) As discussed also in Atuin, bash-preexec.sh tries to offer the preexec hook as much as possible, but still, it's not perfect and robust. It shouldn't be forcibly enabled globally. Remember that Fedora is the upstream of RHEL, which is widely used in shared systems such as the HPC systems. Even if it is not automatically loaded through /etc/profile.d, the bash-preexec.sh package is still useful so that the users who want it can simply write source /usr/share/bash-preexec/bash-preexec.sh in their .bashrc.

Honestly, the atuin package shouldn't be forcibly enabled by /etc/profile.d either. There should be an option that allows installing Atuin through the package manager, yet the users can choose whether to use it at their discretion. I'm not sure about the convention in Fedora, but one of the typical solutions is to prepare two separate packages, atuin and atuin-bash-integration, where the former just installs Atuin binary, and the latter installs the loader of $(atuin init bash) in /etc/profile.d along with the dependency on the atuin package.

The script bash-preexec.sh may be loaded by the atuin loader in /etc/profile.d. However, bash-preexec.sh is merely one option for using Atuin in Bash. Users can also combine Atuin with ble.sh. In that sense, it is strange that just installing the atuin package forces bash-preexec.sh. Then, one could prepare three packages atuin, atuin-bash-integration, and atuin-blesh-integration, where the second one loads $(atuin init bash) and bash-preexec.sh and the third one loads $(atuin init bash) and ble.sh.

but for security concerns I want to gate it to only be sourced if it's in an interactive environment.

Such a check should always be performed. In addition, the check for the Bash version is needed because /etc/profile.d/*.sh will be loaded in all the Bourne-like shells (including bash, ksh, mksh, zsh, etc.).

[LecrisUT]

Thank you for the detailed rundown. It will help a lot for the reviewers.

Maybe we can have an explicit check for the interactive shell at the beginning of bash-preexec.sh.

That would be fantastic. Indeed I thought that this had two modes for interactive and non-interactive (and indeed "interactive mode/shell of Bash"). I would probably have went with a random approach reading the stackexchange, but if you know of good POSIX ways to do such a check, or if it can be incorporate inside the script without patching, that would help a lot.

I personally don't think it is a good idea to put the loader in /etc/profile.d. That will forcibly load bash-preexec.sh in all the users in the system including root.

Indeed it's a double-edged sword. We will try to make it as secure as possible and I am pinging some more experienced people for advice as well. I think one good reference is Lmod where they ship a nice configuration that does do the check for root. Would be nice if such a configuration is upstreamed for reference.

I think that some configuration should be done at installation, because we want to provide a good out-of-the-box experience for people installing atuin. Particularly for single-user environments.

Remember that Fedora is the upstream of RHEL, which is widely used in shared systems such as the HPC systems.

Yes, but not all packages are upstreamed to RHEL. Particularly these ones I think will only live in EPEL.

I'm not sure about the convention in Fedora, but one of the typical solutions is to prepare two separate packages, atuin and atuin-bash-integration, where the former just installs Atuin binary, and the latter installs the loader of $(atuin init bash) in /etc/profile.d along with the dependency on the atuin package.

Great suggestion. There is indeed precedence for that with bash-completion sub-packages. I will bring this up at the review. But we could also consider the Lmod approach.

Users can also combine Atuin with ble.sh. In that sense, it is strange that just installing the atuin package forces bash-preexec.sh

For that we have tools, e.g. we can put Requires: bash-preexec or blesh, but the latter I am more reluctant to package. If there is a request I will look into it of course. There's also the support for the other shell environments which I did not research yet. Thankfully atuin does not need to change its script to support one or the other, so just making them backend installed (and maybe conflicting on each other) should be sufficient.

In addition, the check for the Bash version is needed because /etc/profile.d/*.sh will be loaded in all the Bourne-like shells (including bash, ksh, mksh, zsh, etc.).

Indeed, we were considering 2 aspects, the master branch has the appropriate bash environment and version checks, so we will probably go for packaging a newer commit, until a release is cut. And there was the recommendation to rename the file to bash-preexec.bash (and the accompanying wrapper if we go with that approach).

---

Again, thank you so much for your comments, it helps a lot :slightly_smiling_face:

[akinomyoga]

Maybe we can have an explicit check for the interactive shell at the beginning of bash-preexec.sh.

[...], but if you know of good POSIX ways to do such a check

A commonly used way compatible with POSIX is case $- in *i*) ;; *) return 0; esac, but we don't need to stick with POSIX because we require Bash anyway. One can perform the check for the interactive shell after the Bash version check.

or if it can be incorporate inside the script without patching, that would help a lot.

160, (though I'm not sure if it would be merged. The bash-preexec maintainer seems to require reviewers, but recently no one seems to review the PRs).

In addition, the check for the Bash version is needed because /etc/profile.d/*.sh will be loaded in all the Bourne-like shells (including bash, ksh, mksh, zsh, etc.).

Indeed, we were considering 2 aspects, the master branch has the appropriate bash environment and version checks, so we will probably go for packaging a newer commit, until a release is cut.

Could you also take care of Atuin's /etc/profile.d/atuin.sh? It currently seems just a dump of $(atuin init bash), but I suspect this would cause an error message when /etc/profile is sourced by ksh.

[LecrisUT]

160, (though I'm not sure if it would be merged. The bash-preexec maintainer seems to require reviewers, but recently no one seems to review the PRs).

Thanks :pray:. I can ask whoever picks up the package for review to also chime in on the PR and at least give some feedback then. @juhp gave me great comments on the Fedora-devel, but not sure how free they are.

Could you also take care of Atuin's /etc/profile.d/atuin.sh? It currently seems just a dump of $(atuin init bash), but I suspect this would cause an error message when /etc/profile is sourced by ksh.

On the packaging side? Sure, I can cook something up. But maybe also worth figuring out a convention upstream, e.g. similar checks for root and overrides.