diff --git a/internal/boxcli/shellenv.go b/internal/boxcli/shellenv.go index a6b61e2ae4e..eefd4c83fd5 100644 --- a/internal/boxcli/shellenv.go +++ b/internal/boxcli/shellenv.go @@ -149,9 +149,10 @@ func shellEnvFunc( Pure: flags.pure, SkipRecompute: !flags.recomputeEnv, }, - NoRefreshAlias: flags.noRefreshAlias, - RunHooks: flags.runInitHook, - ShellFormat: shellFormat, + NoRefreshAlias: flags.noRefreshAlias, + OnlyModifiedEnv: true, + RunHooks: flags.runInitHook, + ShellFormat: shellFormat, }) if err != nil { return "", err diff --git a/internal/devbox/devbox.go b/internal/devbox/devbox.go index 4c3f82421ce..b12af32c008 100644 --- a/internal/devbox/devbox.go +++ b/internal/devbox/devbox.go @@ -379,6 +379,16 @@ func (d *Devbox) EnvExports(ctx context.Context, opts devopt.EnvExportsOpts) (st return "", err } + // For `devbox shellenv`, only emit the variables that Devbox actually adds + // or changes relative to the current shell. Re-exporting unrelated variables + // (e.g. HOSTNAME, LANG) is redundant, and can fail when the user's shell + // marks some of them read-only (e.g. PROFILEREAD on openSUSE). See #2826. + // In pure mode we keep the full environment, since the goal there is a + // complete, self-contained environment rather than a diff. + if opts.OnlyModifiedEnv && !opts.EnvOptions.Pure { + envs = onlyModifiedEnvVars(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/devopt/devboxopts.go b/internal/devbox/devopt/devboxopts.go index 9afeaccc4c1..4d575d264bf 100644 --- a/internal/devbox/devopt/devboxopts.go +++ b/internal/devbox/devopt/devboxopts.go @@ -79,8 +79,15 @@ const ( type EnvExportsOpts struct { EnvOptions EnvOptions NoRefreshAlias bool - RunHooks bool - ShellFormat ShellFormat + // OnlyModifiedEnv restricts the exported variables to those that Devbox adds + // or changes relative to the current environment. It is used by + // `devbox shellenv` so it does not re-export unrelated (and possibly + // read-only) variables. It is ignored in pure mode, where the intent is to + // emit a complete, self-contained environment rather than a diff against the + // current shell. + OnlyModifiedEnv bool + RunHooks bool + ShellFormat ShellFormat } // EnvOptions configure the Devbox Environment in the `computeEnv` function. diff --git a/internal/devbox/envvars.go b/internal/devbox/envvars.go index 307546f30e0..4e0ebbbda10 100644 --- a/internal/devbox/envvars.go +++ b/internal/devbox/envvars.go @@ -164,6 +164,22 @@ func exportifyNushell(w io.Writer, vars map[string]string) string { return strings.TrimSpace(strb.String()) } +// onlyModifiedEnvVars returns the subset of env whose values are new or differ +// from the ambient environment. Variables whose value already matches the +// ambient environment are omitted: re-exporting them is redundant, and at worst +// it breaks `eval "$(devbox shellenv)"` when the user's shell marks some of +// those variables read-only (e.g. PROFILEREAD on openSUSE, which produces +// "read-only variable: PROFILEREAD"). See issue #2826. +func onlyModifiedEnvVars(env, ambient map[string]string) map[string]string { + modified := make(map[string]string, len(env)) + for key, val := range env { + if ambientVal, ok := ambient[key]; !ok || ambientVal != val { + modified[key] = val + } + } + return modified +} + // 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, diff --git a/internal/devbox/envvars_test.go b/internal/devbox/envvars_test.go index d387feb8c19..c0adfc7cd06 100644 --- a/internal/devbox/envvars_test.go +++ b/internal/devbox/envvars_test.go @@ -47,6 +47,46 @@ func TestExportifySkipsInvalidNames(t *testing.T) { } } +// TestOnlyModifiedEnvVars ensures that variables identical to the ambient +// environment are dropped while new or changed variables are kept. This is what +// keeps `devbox shellenv` from re-exporting unrelated, possibly read-only +// variables (see issue #2826). +func TestOnlyModifiedEnvVars(t *testing.T) { + ambient := map[string]string{ + "HOSTNAME": "myhost", + "LANG": "en_US.UTF-8", + "PROFILEREAD": "true", + "PATH": "/usr/bin:/bin", + } + env := map[string]string{ + "HOSTNAME": "myhost", // unchanged -> dropped + "LANG": "en_US.UTF-8", // unchanged -> dropped + "PROFILEREAD": "true", // unchanged (read-only) -> dropped + "PATH": "/devbox/bin:/usr/bin", // changed -> kept + "DEVBOX_PROJECT_ROOT": "/home/user/proj", // new -> kept + } + + got := onlyModifiedEnvVars(env, ambient) + + want := map[string]string{ + "PATH": "/devbox/bin:/usr/bin", + "DEVBOX_PROJECT_ROOT": "/home/user/proj", + } + if len(got) != len(want) { + t.Fatalf("onlyModifiedEnvVars returned %d vars, want %d: %v", len(got), len(want), got) + } + for k, v := range want { + if got[k] != v { + t.Errorf("onlyModifiedEnvVars[%q] = %q, want %q", k, got[k], v) + } + } + for _, dropped := range []string{"HOSTNAME", "LANG", "PROFILEREAD"} { + if _, ok := got[dropped]; ok { + t.Errorf("expected unchanged var %q to be dropped, got:\n%v", dropped, got) + } + } +} + func TestExportifyNushellSkipsInvalidNames(t *testing.T) { got := exportifyNushell(io.Discard, map[string]string{ "GOOD": "value",