From b1d1a3e4a8848f523bbe42c56b02e2e20518e4f1 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 4 Jul 2026 14:12:02 +0000 Subject: [PATCH] fix(shellenv): only export env vars Devbox actually modifies `devbox shellenv` and `devbox global shellenv` emitted an `export` statement for every variable in the computed environment, including inherited variables that Devbox never changed (e.g. HOSTNAME, LANG, PROFILEREAD). Re-exporting an unchanged variable is redundant, and it breaks in shells that mark some inherited variables as read-only: on openSUSE, `eval "$(devbox global shellenv)"` failed on every prompt with `read-only variable: PROFILEREAD`. Filter the computed environment down to the variables whose value differs from the current environment (or that are new) before formatting the export statements. The output is meant to be eval'd in the caller's shell, so skipping variables that already have the same value is a no-op in the normal case and avoids the read-only error. The interactive `devbox shell` code path (which sources a generated shellrc in a child shell) is unchanged; only `EnvExports`, used by `devbox shellenv` and `devbox shell --print-env`, is affected. Fixes #2826 Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01WiTxPixVaWVbpBsZf1DLeG --- internal/devbox/devbox.go | 9 ++++ internal/devbox/envvars.go | 21 ++++++++ internal/devbox/envvars_test.go | 88 +++++++++++++++++++++++++++++++++ 3 files changed, 118 insertions(+) create mode 100644 internal/devbox/envvars_test.go diff --git a/internal/devbox/devbox.go b/internal/devbox/devbox.go index 8ac001233d3..90f68df2dc7 100644 --- a/internal/devbox/devbox.go +++ b/internal/devbox/devbox.go @@ -379,6 +379,15 @@ func (d *Devbox) EnvExports(ctx context.Context, opts devopt.EnvExportsOpts) (st return "", err } + // Only export the variables that Devbox actually adds or changes relative to + // the current environment. The output of this function is meant to be eval'd + // in the caller's shell (e.g. `eval "$(devbox shellenv)"`), so re-exporting + // variables that are already set to the same value is unnecessary and can + // fail in shells that mark some inherited variables as read-only (e.g. + // PROFILEREAD on openSUSE). See + // https://github.com/jetify-com/devbox/issues/2826. + envs = onlyModifiedEnvs(envs, envir.PairsToMap(os.Environ())) + // Use the appropriate export format based on shell type var envStr string if opts.ShellFormat == devopt.ShellFormatNushell { diff --git a/internal/devbox/envvars.go b/internal/devbox/envvars.go index ec8231e19ab..1643511209a 100644 --- a/internal/devbox/envvars.go +++ b/internal/devbox/envvars.go @@ -14,6 +14,27 @@ import ( const devboxSetPrefix = "__DEVBOX_SET_" +// onlyModifiedEnvs returns the subset of envs whose values differ from those in +// baseEnv (or that are absent from baseEnv entirely). +// +// It is used to avoid emitting `export` statements for variables that Devbox did +// not actually change. Re-exporting a variable with the value it already has is +// not only redundant, it can break shells that mark some inherited variables as +// read-only. For example, openSUSE marks PROFILEREAD read-only, so +// `eval "$(devbox global shellenv)"` failed with +// "read-only variable: PROFILEREAD". See +// https://github.com/jetify-com/devbox/issues/2826. +func onlyModifiedEnvs(envs, baseEnv map[string]string) map[string]string { + modified := make(map[string]string, len(envs)) + for k, v := range envs { + if base, ok := baseEnv[k]; ok && base == v { + continue + } + modified[k] = v + } + return modified +} + // 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 diff --git a/internal/devbox/envvars_test.go b/internal/devbox/envvars_test.go new file mode 100644 index 00000000000..1ec55cc3c6e --- /dev/null +++ b/internal/devbox/envvars_test.go @@ -0,0 +1,88 @@ +// Copyright 2024 Jetify Inc. and contributors. All rights reserved. +// Use of this source code is governed by the license in the LICENSE file. + +package devbox + +import ( + "maps" + "testing" +) + +func TestOnlyModifiedEnvs(t *testing.T) { + tests := []struct { + name string + envs map[string]string + baseEnv map[string]string + want map[string]string + }{ + { + name: "empty", + envs: map[string]string{}, + baseEnv: map[string]string{}, + want: map[string]string{}, + }, + { + name: "drops unchanged variables", + envs: map[string]string{ + "HOSTNAME": "myhost", + "LANG": "en_US.UTF-8", + "PROFILEREAD": "true", + }, + baseEnv: map[string]string{ + "HOSTNAME": "myhost", + "LANG": "en_US.UTF-8", + "PROFILEREAD": "true", + }, + want: map[string]string{}, + }, + { + name: "keeps changed and new variables", + envs: map[string]string{ + "PATH": "/nix/store/bin:/usr/bin", // changed + "HOSTNAME": "myhost", // unchanged + "DEVBOX_PROJECT_ROOT": "/home/user/project", // new + }, + baseEnv: map[string]string{ + "PATH": "/usr/bin", + "HOSTNAME": "myhost", + }, + want: map[string]string{ + "PATH": "/nix/store/bin:/usr/bin", + "DEVBOX_PROJECT_ROOT": "/home/user/project", + }, + }, + { + name: "keeps variable that became empty", + envs: map[string]string{ + "FOO": "", + }, + baseEnv: map[string]string{ + "FOO": "bar", + }, + want: map[string]string{ + "FOO": "", + }, + }, + { + name: "empty base env keeps everything", + envs: map[string]string{ + "FOO": "bar", + "BAZ": "qux", + }, + baseEnv: map[string]string{}, + want: map[string]string{ + "FOO": "bar", + "BAZ": "qux", + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + got := onlyModifiedEnvs(test.envs, test.baseEnv) + if !maps.Equal(got, test.want) { + t.Errorf("onlyModifiedEnvs() = %v, want %v", got, test.want) + } + }) + } +}