Skip to content

impl: set SHELL environment variable #1512

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 29, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion internal/boxcli/integrate.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ import (
"bytes"
"encoding/json"
"os/exec"
"slices"
"strings"

"github.com/spf13/cobra"
"github.com/zealic/go2node"
Expand Down Expand Up @@ -54,7 +56,6 @@ type parentMessage struct {
}

func runIntegrateVSCodeCmd(cmd *cobra.Command) error {

// Setup process communication with node as parent
channel, err := go2node.RunAsNodeChild()
if err != nil {
Expand Down Expand Up @@ -85,6 +86,14 @@ func runIntegrateVSCodeCmd(cmd *cobra.Command) error {
if err != nil {
return err
}
envVars = slices.DeleteFunc(envVars, func(s string) bool {
k, _, ok := strings.Cut(s, "=")
// DEVBOX_OG_PATH_<hash> being set causes devbox global shellenv to overwrite the
// PATH after VSCode opens and resets it to global shellenv. This causes the VSCode
// terminal to not be able to find devbox packages after the reopen in devbox
// environment action is called.
return ok && (strings.HasPrefix(k, "DEVBOX_OG_PATH") || k == "HOME" || k == "NODE_CHANNEL_FD")
})

// Send message to parent process to terminate
err = channel.Write(&go2node.NodeMessage{
Expand Down
4 changes: 1 addition & 3 deletions internal/impl/devbox.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,6 @@ type Devbox struct {
var legacyPackagesWarningHasBeenShown = false

func Open(opts *devopt.Opts) (*Devbox, error) {

projectDir, err := findProjectDir(opts.Dir)
if err != nil {
return nil, err
Expand Down Expand Up @@ -318,7 +317,7 @@ func (d *Devbox) EnvVars(ctx context.Context) ([]string, error) {
if err != nil {
return nil, err
}
return keyEqualsValue(envs), nil
return mapToPairs(envs), nil
}

func (d *Devbox) ShellEnvHash(ctx context.Context) (string, error) {
Expand Down Expand Up @@ -684,7 +683,6 @@ func (d *Devbox) StartProcessManager(
background bool,
processComposeFileOrDir string,
) error {

if !d.IsEnvEnabled() {
args := []string{"services", "up"}
args = append(args, requestedServices...)
Expand Down
82 changes: 22 additions & 60 deletions internal/impl/envvars.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,24 +4,31 @@
package impl

import (
"fmt"
"os"
"sort"
"slices"
"strings"
)

const devboxSetPrefix = "__DEVBOX_SET_"

// mapToPairs creates a slice of environment variable "key=value" pairs from a
// map.
func mapToPairs(m map[string]string) []string {
pairs := []string{}
pairs := make([]string, len(m))
i := 0
for k, v := range m {
pairs = append(pairs, fmt.Sprintf("%s=%s", k, v))
pairs[i] = k + "=" + v
i++
}
slices.Sort(pairs) // for reproducibility
return pairs
}

// pairsToMap creates a map from a slice of "key=value" environment variable
// pairs. Note that maps are not ordered, which can affect the final variable
// values when pairs contains duplicate keys.
func pairsToMap(pairs []string) map[string]string {
vars := map[string]string{}
vars := make(map[string]string, len(pairs))
for _, p := range pairs {
k, v, ok := strings.Cut(p, "=")
if !ok {
Expand All @@ -32,14 +39,19 @@ func pairsToMap(pairs []string) map[string]string {
return vars
}

// exportify takes a map of [string]string and returns a single string
// of the form export KEY="VAL"; and escapes all the vals from special characters.
// exportify formats vars as a line-separated string of shell export statements.
// Each line is of the form `export key="value";` with any special characters in
// value escaped. This means that the shell will always interpret values as
// literal strings; no variable expansion or command substitution will take
// place.
func exportify(vars map[string]string) string {
keys := make([]string, 0, len(vars))
keys := make([]string, len(vars))
i := 0
for k := range vars {
keys = append(keys, k)
keys[i] = k
i++
}
sort.Strings(keys)
slices.Sort(keys) // for reproducibility

strb := strings.Builder{}
for _, k := range keys {
Expand All @@ -60,56 +72,6 @@ func exportify(vars map[string]string) string {
return strings.TrimSpace(strb.String())
}

// exportify takes a map of [string]string and returns an array of string
// of the form KEY="VAL" and escapes all the vals from special characters.
func keyEqualsValue(vars map[string]string) []string {
keys := make([]string, 0, len(vars))
for k := range vars {
keys = append(keys, k)
}
keyValues := make([]string, 0, len(vars))
sort.Strings(keys)

for _, k := range keys {
if isApproved(k) {
strb := strings.Builder{}
strb.WriteString(k)
strb.WriteString(`=`)
for _, r := range vars[k] {
switch r {
// Special characters inside double quotes:
// https://pubs.opengroup.org/onlinepubs/009604499/utilities/xcu_chap02.html#tag_02_02_03
case '$', '`', '"', '\\', '\n':
strb.WriteRune('\\')
}
strb.WriteRune(r)
}
keyValues = append(keyValues, strb.String())
}
}
return keyValues
}

func isApproved(key string) bool {
// list to keys
// should find the corrupt key
troublingEnvKeys := []string{
"HOME",
"NODE_CHANNEL_FD",
}
approved := true
for _, ak := range troublingEnvKeys {
// DEVBOX_OG_PATH_<hash> being set causes devbox global shellenv or overwrite
// the PATH after vscode opens and resets it to global shellenv
// This causes vscode terminal to not be able to find devbox packages
// after reopen in devbox environment action is called
if key == ak || strings.HasPrefix(key, "DEVBOX_OG_PATH") {
approved = false
}
}
return approved
}

// addEnvIfNotPreviouslySetByDevbox adds the key-value pairs from new to existing,
// but only if the key was not previously set by devbox
// Caveat, this won't mark the values as set by devbox automatically. Instead,
Expand Down
1 change: 1 addition & 0 deletions internal/impl/shell.go
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,7 @@ func (s *DevboxShell) Run() error {
for k, v := range extraEnv {
env[k] = v
}
env["SHELL"] = s.binPath

cmd = exec.Command(s.binPath)
cmd.Env = mapToPairs(env)
Expand Down
2 changes: 1 addition & 1 deletion internal/impl/shell_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ func testWriteDevboxShellrc(t *testing.T, testdirs []string) {
// set, and then proceed normally. The test should
// always pass in this case.
if *update {
err = os.WriteFile(test.goldShellrcPath, gotShellrc, 0666)
err = os.WriteFile(test.goldShellrcPath, gotShellrc, 0o666)
if err != nil {
t.Error("Error updating golden files:", err)
}
Expand Down