Drop ambient copies of vctx-managed env vars before exec

docs/vctx.md

@@ -32,7 +32,7 @@ vctx --context sydney kv put kv/foo/bar secret=baz
    context. Because it replaces the process, `vault`'s exit status, signals and
    TTY behaviour pass straight through.
 
-The ambient `VAULT_ADDR` / `VAULT_TOKEN` in your shell are ignored for the call
+The ambient `VAULT_ADDR` / `VAULT_TOKEN` / `VAULT_NAMESPACE` in your shell are ignored for the call
 — `vctx` always targets the chosen context.
 
 ## Output format
@@ -48,7 +48,7 @@ Without `--format`, `vctx` sets `VAULT_FORMAT` from the vault command:
 
 It is left alone when `VAULT_FORMAT` is already set in the environment or the
 vault args include `-format` or `-field`. `--format` (`table`, `json`, `yaml`,
-`pretty`; `raw` for `vault read` only) always sets it.
+`pretty`; `raw` for `vault read` only) always sets it, overriding the environment.
 
 ## Flags
 

vctx/main.go

@@ -106,9 +106,10 @@ func runVctx(context, format string, args []string) error {
 // buildInvocation resolves the context and its cached token, locates the vault
 // binary, and returns the binary path, argv (vault + args) and the environment
 // to exec with. baseEnv is the starting environment (normally os.Environ());
-// the Vault settings are appended so they override any ambient values (exec
-// semantics: the last assignment of a variable wins). format, when non-empty,
-// is set as VAULT_FORMAT; otherwise autoFormat picks one.
+// ambient copies of the keys vctx sets are dropped first, since Go programs
+// (vault included) honour the first occurrence of a duplicated variable.
+// VAULT_NAMESPACE is always dropped so a context without one is not redirected.
+// format, when non-empty, is set as VAULT_FORMAT; otherwise autoFormat picks one.
 func buildInvocation(context, format string, args, baseEnv []string) (bin string, argv []string, env []string, err error) {
 	cfg, err := shared.Load()
 	if err != nil {
@@ -128,12 +129,10 @@ func buildInvocation(context, format string, args, baseEnv []string) (bin string
 		return "", nil, nil, fmt.Errorf("vault CLI not found in PATH: %w", err)
 	}
 
-	env = make([]string, len(baseEnv), len(baseEnv)+4)
-	copy(env, baseEnv)
-	env = append(env,
-		"VAULT_ADDR="+rc.Address,
-		"VAULT_TOKEN="+tok.Token,
-	)
+	set := map[string]string{
+		"VAULT_ADDR":  rc.Address,
+		"VAULT_TOKEN": tok.Token,
+	}
 	// Namespace comes from the context, falling back to what the token was
 	// issued under; only set when non-empty.
 	ns := rc.Namespace
@@ -141,14 +140,26 @@ func buildInvocation(context, format string, args, baseEnv []string) (bin string
 		ns = tok.Namespace
 	}
 	if ns != "" {
-		env = append(env, "VAULT_NAMESPACE="+ns)
+		set["VAULT_NAMESPACE"] = ns
 	}
-
 	if format == "" {
 		format = autoFormat(args, baseEnv)
 	}
 	if format != "" {
-		env = append(env, "VAULT_FORMAT="+format)
+		set["VAULT_FORMAT"] = format
+	}
+
+	for _, e := range baseEnv {
+		k, _, _ := strings.Cut(e, "=")
+		if _, ok := set[k]; ok || k == "VAULT_NAMESPACE" {
+			continue
+		}
+		env = append(env, e)
+	}
+	for _, k := range []string{"VAULT_ADDR", "VAULT_TOKEN", "VAULT_NAMESPACE", "VAULT_FORMAT"} {
+		if v, ok := set[k]; ok {
+			env = append(env, k+"="+v)
+		}
 	}
 
 	argv = append([]string{vaultBin}, args...)

vctx/main_test.go

@@ -53,16 +53,20 @@ contexts:
     namespace: staging
 `
 
-// lastVal returns the value of the last occurrence of key in an env slice,
-// mirroring exec's "last assignment wins" semantics.
-func lastVal(env []string, key string) (string, bool) {
-	val, ok := "", false
+// envVal returns key's value from an env slice, failing the test if the key
+// appears more than once (Go's os/exec children honour the first occurrence).
+func envVal(t *testing.T, env []string, key string) (string, bool) {
+	t.Helper()
+	val, n := "", 0
 	for _, e := range env {
 		if strings.HasPrefix(e, key+"=") {
-			val, ok = e[len(key)+1:], true
+			val, n = e[len(key)+1:], n+1
 		}
 	}
-	return val, ok
+	if n > 1 {
+		t.Errorf("%s appears %d times in env %v", key, n, env)
+	}
+	return val, n > 0
 }
 
 func TestBuildInvocationArgsAndEnvOverride(t *testing.T) {
@@ -71,7 +75,7 @@ func TestBuildInvocationArgsAndEnvOverride(t *testing.T) {
 	})
 
 	// Ambient VAULT_ADDR/VAULT_TOKEN must be overridden by the context's values.
-	baseEnv := []string{"HOME=/home/x", "VAULT_ADDR=ambient", "VAULT_TOKEN=ambient"}
+	baseEnv := []string{"HOME=/home/x", "VAULT_ADDR=ambient", "VAULT_TOKEN=ambient", "VAULT_NAMESPACE=ambient"}
 	args := []string{"kv", "get", "-field=foo", "secret/x"}
 
 	bin, argv, env, err := buildInvocation("sydney", "", args, baseEnv)
@@ -86,17 +90,17 @@ func TestBuildInvocationArgsAndEnvOverride(t *testing.T) {
 	if !reflect.DeepEqual(argv, wantArgv) {
 		t.Errorf("argv = %v, want %v", argv, wantArgv)
 	}
-	if v, _ := lastVal(env, "VAULT_ADDR"); v != "https://vault.syd1.au.unkin.net" {
+	if v, _ := envVal(t, env, "VAULT_ADDR"); v != "https://vault.syd1.au.unkin.net" {
 		t.Errorf("VAULT_ADDR = %q, want context override to win", v)
 	}
-	if v, _ := lastVal(env, "VAULT_TOKEN"); v != "s.SYDTOK" {
+	if v, _ := envVal(t, env, "VAULT_TOKEN"); v != "s.SYDTOK" {
 		t.Errorf("VAULT_TOKEN = %q, want s.SYDTOK", v)
 	}
-	if _, ok := lastVal(env, "VAULT_NAMESPACE"); ok {
+	if _, ok := envVal(t, env, "VAULT_NAMESPACE"); ok {
 		t.Error("VAULT_NAMESPACE should be unset for a context with no namespace")
 	}
 	// The ambient HOME must be preserved.
-	if v, _ := lastVal(env, "HOME"); v != "/home/x" {
+	if v, _ := envVal(t, env, "HOME"); v != "/home/x" {
 		t.Errorf("HOME = %q, base env not preserved", v)
 	}
 }
@@ -109,7 +113,7 @@ func TestBuildInvocationNamespaceFromContext(t *testing.T) {
 	if err != nil {
 		t.Fatalf("buildInvocation: %v", err)
 	}
-	if v, _ := lastVal(env, "VAULT_NAMESPACE"); v != "staging" {
+	if v, _ := envVal(t, env, "VAULT_NAMESPACE"); v != "staging" {
 		t.Errorf("VAULT_NAMESPACE = %q, want staging", v)
 	}
 }
@@ -124,7 +128,7 @@ func TestBuildInvocationNamespaceFallsBackToToken(t *testing.T) {
 	if err != nil {
 		t.Fatalf("buildInvocation: %v", err)
 	}
-	if v, _ := lastVal(env, "VAULT_NAMESPACE"); v != "from-token" {
+	if v, _ := envVal(t, env, "VAULT_NAMESPACE"); v != "from-token" {
 		t.Errorf("VAULT_NAMESPACE = %q, want from-token fallback", v)
 	}
 }
@@ -179,7 +183,7 @@ func TestRunVctxExecsWithBuiltInvocation(t *testing.T) {
 	if !reflect.DeepEqual(gotArgv, []string{vault, "kv", "list", "kv/"}) {
 		t.Errorf("exec argv = %v", gotArgv)
 	}
-	if v, _ := lastVal(gotEnv, "VAULT_TOKEN"); v != "s.SYDTOK" {
+	if v, _ := envVal(t, gotEnv, "VAULT_TOKEN"); v != "s.SYDTOK" {
 		t.Errorf("exec env VAULT_TOKEN = %q", v)
 	}
 }
@@ -229,24 +233,28 @@ func TestBuildInvocationFormat(t *testing.T) {
 	setupVctx(t, vctxConfig, map[string]shared.Token{
 		"sydney": {Address: "https://vault.syd1.au.unkin.net", Token: "s.X"},
 	})
+	ambient := []string{"VAULT_FORMAT=table"}
 	cases := []struct {
 		flag string
 		args []string
+		base []string
 		want string
 		set  bool
 	}{
-		{"", []string{"kv", "get", "kv/foo"}, "yaml", true},
-		{"", []string{"status"}, "", false},
-		{"json", []string{"kv", "get", "kv/foo"}, "json", true},
-		{"json", []string{"status"}, "json", true},
+		{"", []string{"kv", "get", "kv/foo"}, nil, "yaml", true},
+		{"", []string{"status"}, nil, "", false},
+		{"json", []string{"kv", "get", "kv/foo"}, nil, "json", true},
+		{"json", []string{"status"}, nil, "json", true},
+		{"", []string{"kv", "get", "kv/foo"}, ambient, "table", true},
+		{"json", []string{"kv", "get", "kv/foo"}, ambient, "json", true},
 	}
 	for _, c := range cases {
-		_, _, env, err := buildInvocation("sydney", c.flag, c.args, nil)
+		_, _, env, err := buildInvocation("sydney", c.flag, c.args, c.base)
 		if err != nil {
 			t.Fatalf("buildInvocation: %v", err)
 		}
-		if v, ok := lastVal(env, "VAULT_FORMAT"); v != c.want || ok != c.set {
-			t.Errorf("flag %q args %v: VAULT_FORMAT = %q (set %v), want %q (set %v)", c.flag, c.args, v, ok, c.want, c.set)
+		if v, ok := envVal(t, env, "VAULT_FORMAT"); v != c.want || ok != c.set {
+			t.Errorf("flag %q args %v base %v: VAULT_FORMAT = %q (set %v), want %q (set %v)", c.flag, c.args, c.base, v, ok, c.want, c.set)
 		}
 	}
 }