opam icon indicating copy to clipboard operation
opam copied to clipboard

Set environment variables needed for bash, zsh completion in opam env

Open WardBrian opened this issue 1 month ago • 5 comments

Would close #6427

This updates opam env to set XDG_DATA_DIRS (from the XDG base directory specification) to include the current switch's share root, and FPATH to include %{share}%/zsh/site-functions when the shell is zsh

The former works with bash-completion, which looks in the XDG_DATA_DIRS entries for bash-completion/completions sub-directories, while the later sets up zsh's built-in completion mechanisms to look in this location.

Questions for reviewers --

  1. I currently only modify FPATH on zsh, but I think it would be safe to do it generically (the only other shell that seems to assign meaning to the FPATH variable is ksh, which opam doesn't support)
  2. Assuming we want to keep FPATH zsh-specific, is it worth threading through a shell variable all the way from the CLI to where we compute these updates, or is it fine to keep the guess_shell_compat call?

WardBrian avatar Nov 24 '25 17:11 WardBrian

cc @hannesm @reynir do you have any thoughts about the security considerations of this feature by any chance?

kit-ty-kate avatar Dec 01 '25 16:12 kit-ty-kate

@kit-ty-kate thanks for asking, no I don't have an idea about this environment variable (and whether to prepend or append to it).

hannesm avatar Dec 02 '25 12:12 hannesm

I don't know much about it. For bash-completion I suspect it could add completion scripts from installed opam packages, and I think those can run arbitrary commands. Then again I guess packages can already install arbitrary binaries shadowing common binaries such as ls. I'm not sure what the security model is or how that fits into that.

reynir avatar Dec 04 '25 16:12 reynir

Even if you're sandboxing the build commands (so you can't be pwned by arbitrary badness hidden behind a make command), I still think it is currently the case that you can't install an opam package unless you trust it. It doesn't even have to shadow something like ls; presumably you're going to run the code from the package eventually, or else you wouldn't be installing it.

WardBrian avatar Dec 04 '25 16:12 WardBrian

Then again I guess packages can already install arbitrary binaries shadowing common binaries such as ls

Not to mention that arbitrary ocaml packages can actually set or extend theses variables themselves…

dbuenzli avatar Dec 04 '25 16:12 dbuenzli

Not to mention that arbitrary ocaml packages can actually set or extend theses variables themselves…

Yes. If this is decided out of scope for the opam tool, possibly the next best option would be a conf-completion package in the opam repository which contains just these two variables as setenv fields. Then tools could depend on that to get their users environments correctly configured for autocomplete.

That feels like a bit of a hack compared to doing it in code here.

WardBrian avatar Dec 17 '25 00:12 WardBrian