Skip to content

Commit

Permalink
shell,nix: fix shell PATH to prefer nix packages (#51)
Browse files Browse the repository at this point in the history
This fixes an issue where the user's rc (~/.bashrc, ~/.zshrc, etc.)
files were prepending to the devbox shell PATH. This can cause the
shell to unexpectedly run the non-devbox version of programs. It's
especially noticeable with some version managers that point the PATH at
their shims so they can control which version of a program is run.

The basic steps that result in this issue are:

1. Devbox launches Nix, which sets the PATH to point to the Nix store.
2. From within Nix, we launch the user's default shell.
3. The shell runs the user's init files and stomps on the Nix PATH.

Fixing this is a little tricky because there's no straightforward way to
run additional commands _after_ the shell runs the init files. Instead,
we need to copy the user's init files and append to them. Doing this
requires knowing how each shell handles initialization, so we need to
special-case the more common shells by performing the following steps:

1. Attempt to detect the user's default shell via the SHELL env var (we
   may add more sophisticated detection later).
2. If the shell is recognized as bash, zsh, ksh, dash, ash or sh, copy
   over the user's corresponding init files to a temp directory and
   append our own commands to it.
3. Build an `exec` shell command that invokes the new shell and tells it
   to run our temp init file instead of the user's.

If we can't detect the shell, we fall back to launching a vanilla Nix
shell. If we don't recognize the detected shell, we fall back to
launching it without overriding the init files.

Finally, since we now have a way of running our own commands in the
user's shell, we can change the PS1 prompt to show the user that
they're in devbox.

Fixes #17, #25, #44, #46.
  • Loading branch information
gcurtis committed Sep 1, 2022
1 parent 36333f9 commit 485e3a4
Show file tree
Hide file tree
Showing 4 changed files with 207 additions and 10 deletions.
9 changes: 5 additions & 4 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,20 +19,19 @@ linters:
- stylecheck
- typecheck
- unconvert
- unparam
- unparam
- unused
- usestdlibvars
- varnamelen
# - wrapcheck If we're going to use github.com/pkg/errors we should probably turn this on?

# We'd like to have the following linter enabled, but it's broken for Go
# 1.19 as of golangci-lint v1.48.0. Re-enable it when this issue is
# fixed: https://github.com/golangci/golangci-lint/issues/2649
# - structcheck
issues:
exclude:

linters-settings:
errorlint:
errorf: false
varnamelen:
max-distance: 10
ignore-decls:
Expand All @@ -48,6 +47,8 @@ linters-settings:
- m map[string]int
- ns string
- r *http.Request
- sh *Shell
- sh *shell.Shell
- t testing.T
- w http.ResponseWriter
- w io.Writer
54 changes: 49 additions & 5 deletions nix/nix.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,59 @@ import (
"os"
"os/exec"
"strings"

"go.jetpack.io/devbox/shell"
)

func Shell(path string) error {
cmd := exec.Command("nix-shell", path)
// Default to the shell already being used.
shell := os.Getenv("SHELL")
if shell != "" {
cmd.Args = append(cmd.Args, "--command", shell)
// nix-shell only runs bash, which isn't great if the user has a
// different default shell. Here we try to detect what their current
// shell is, and then `exec` it to replace the bash process inside
// nix-shell.
sh, err := shell.Detect()
if err != nil {
// Fall back to running the vanilla Nix bash shell.
return runFallbackShell(path)
}

// Naively running the user's shell has two problems:
//
// 1. The shell will source the user's rc file and potentially reorder
// the PATH. This is especially a problem with some shims that prepend
// their own directories to the front of the PATH, replacing the
// Nix-installed packages.
// 2. If their shell is bash, we end up double-sourcing their ~/.bashrc.
// Once when nix-shell launches bash, and again when we exec it.
//
// To workaround this, first we store the current (outside of devbox)
// PATH in ORIGINAL_PATH. Then we run a "pure" nix-shell to prevent it
// from sourcing their ~/.bashrc. From inside the nix-shell (but before
// launching the user's preferred shell) we store the PATH again in
// PURE_NIX_PATH. When we're finally in the user's preferred shell, we
// can use these env vars to set the PATH so that Nix packages are up
// front, and all of the other programs come after.
//
// ORIGINAL_PATH is set by sh.StartCommand.
// PURE_NIX_PATH is set by the shell hook in shell.nix.tmpl.
_ = sh.SetInit(`
# Update the $PATH so the user can keep using programs that live outside of Nix,
# but prefer anything installed by Nix.
export PATH="$PURE_NIX_PATH:$ORIGINAL_PATH"
# Prepend to the prompt to make it clear we're in a devbox shell.
export PS1="(devbox) $PS1"
`)

cmd := exec.Command("nix-shell", path)
cmd.Args = append(cmd.Args, "--pure", "--command", sh.ExecCommand())
cmd.Stdin = os.Stdin
cmd.Stdout = os.Stdout
cmd.Stderr = os.Stderr
return cmd.Run()
}

func runFallbackShell(path string) error {
cmd := exec.Command("nix-shell", path)
cmd.Stdin = os.Stdin
cmd.Stdout = os.Stdout
cmd.Stderr = os.Stderr
Expand Down
144 changes: 144 additions & 0 deletions shell/shell.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,144 @@
// Copyright 2022 Jetpack Technologies Inc and contributors. All rights reserved.
// Use of this source code is governed by the license in the LICENSE file.

// Package shell detects the user's default shell and configures it to run in
// Devbox.
package shell

import (
"bytes"
"errors"
"fmt"
"os"
"path/filepath"
"strings"
)

type name string

const (
shUnknown name = ""
shBash name = "bash"
shZsh name = "zsh"
shKsh name = "ksh"
shPosix name = "posix"
)

// Shell configures a user's shell to run in Devbox.
type Shell struct {
name name
path string
initFile string
devboxInitFile string
}

// Detect attempts to determine the user's default shell.
func Detect() (*Shell, error) {
path := os.Getenv("SHELL")
if path == "" {
return nil, errors.New("unable to detect the current shell")
}

sh := &Shell{path: filepath.Clean(path)}
base := filepath.Base(path)
// Login shell
if base[0] == '-' {
base = base[1:]
}
switch base {
case "bash":
sh.name = shBash
sh.initFile = rcfilePath(".bashrc")
case "zsh":
sh.name = shZsh
sh.initFile = rcfilePath(".zshrc")
case "ksh":
sh.name = shKsh
sh.initFile = rcfilePath(".kshrc")
case "dash", "ash", "sh":
sh.name = shPosix
sh.initFile = os.Getenv("ENV")

// Just make up a name if there isn't already an init file set
// so we have somewhere to put a new one.
if sh.initFile == "" {
sh.initFile = ".shinit"
}
default:
sh.name = shUnknown
}
return sh, nil
}

// rcfilePath returns the absolute path for an rcfile, which is usually in the
// user's home directory. It doesn't guarantee that the file exists.
func rcfilePath(basename string) string {
home, err := os.UserHomeDir()
if err != nil {
return ""
}
return filepath.Join(home, basename)
}

// SetInit configures the shell to run a script at startup. The script runs
// after the user's usual init files. The script's environment will contain an
// ORIGINAL_PATH environment variable, which will bet set to the PATH before
// the user's init files have had a chance to modify it.
func (s *Shell) SetInit(script string) error {
script = strings.TrimSpace(script)
if script == "" {
return nil
}

initFile, _ := os.ReadFile(s.initFile)
initFile = bytes.TrimSpace(initFile)
if len(initFile) > 0 {
initFile = append(initFile, '\n', '\n')
}

buf := bytes.NewBuffer(initFile)
buf.WriteString(`
# Begin Devbox Shell Hook
`)
buf.WriteString(script)
buf.WriteString(`
# End Devbox Shell Hook
`)

// We need a temp dir (as opposed to a temp file) because zsh uses
// ZDOTDIR to point to a new directory containing the .zshrc.
tmp, err := os.MkdirTemp("", "devbox")
if err != nil {
return fmt.Errorf("create temp dir for shell init file: %v", err)
}
devboxInitFile := filepath.Join(tmp, filepath.Base(s.initFile))
if err := os.WriteFile(devboxInitFile, buf.Bytes(), 0600); err != nil {
return fmt.Errorf("write to shell init file: %v", err)
}
s.devboxInitFile = devboxInitFile
return nil
}

// ExecCommand is a command that replaces the current shell with s.
func (s *Shell) ExecCommand() string {
if s.devboxInitFile == "" {
return "exec " + s.path
}

switch s.name {
case shBash:
return fmt.Sprintf(`exec /usr/bin/env ORIGINAL_PATH="%s" %s --rcfile "%s"`,
os.Getenv("PATH"), s.path, s.devboxInitFile)
case shZsh:
return fmt.Sprintf(`exec /usr/bin/env ORIGINAL_PATH="%s" ZDOTDIR="%s" %s`,
os.Getenv("PATH"), filepath.Dir(s.devboxInitFile), s.path)
case shKsh, shPosix:
return fmt.Sprintf(`exec /usr/bin/env ORIGINAL_PATH="%s" ENV="%s" %s `,
os.Getenv("PATH"), s.devboxInitFile, s.path)
default:
return "exec " + s.path
}
}
10 changes: 9 additions & 1 deletion tmpl/shell.nix.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,18 @@ mkShell {
export name="devbox"
export IN_NIX_SHELL=0
export DEVBOX_SHELL_ENABLED=1

# We set PURE_NIX_PATH in case the user's init files in the devbox shell
# end up modifying PATH.
export PURE_NIX_PATH="$PATH"

# Make sure we include a basic path for the devbox shell. Otherwise it may
# fail to start.
export PATH=$PATH:/usr/bin:/bin:/usr/sbin:/sbin:/usr/local/bin
'';
packages = [
{{- range .Packages}}
{{.}}
{{end -}}
];
}
}

0 comments on commit 485e3a4

Please sign in to comment.