From 1aef0f3f2dd511dc359787bcb71e76a819fc99e3 Mon Sep 17 00:00:00 2001 From: ylorn Date: Sat, 3 Oct 2026 23:13:50 -0400 Subject: [PATCH] =?UTF-8?q?proc:=20the=20login-shell=20probe=20asks=20nush?= =?UTF-8?q?ell=20through=20/bin/sh,=20and=20a=20value=20the=20shell=20prin?= =?UTF-8?q?ted=20instead=20of=20expanding=20counts=20as=20unset=20(#738:?= =?UTF-8?q?=20with=20nushell=20as=20the=20login=20shell,=20shellEnv's=20pr?= =?UTF-8?q?intf=20'=E2=80=A6'=20"$PATH"=20"$CLAUDE=5FCONFIG=5FDIR"=20?= =?UTF-8?q?=E2=80=A6=20came=20back=20as=20the=20text=20"$PATH",=20"$CLAUDE?= =?UTF-8?q?=5FCONFIG=5FDIR",=20=E2=80=A6=20=E2=80=94=20a=20double-quoted?= =?UTF-8?q?=20string=20is=20literal=20in=20nushell=20=E2=80=94=20and=20set?= =?UTF-8?q?Unset=20exported=20every=20one=20of=20agentenv.Vars=20with=20it?= =?UTF-8?q?s=20own=20name=20as=20the=20value;=20Claude=20sign-in=20then=20?= =?UTF-8?q?failed=20with=20"mkdir=20$CLAUDE=5FCONFIG=5FDIR:=20read-only=20?= =?UTF-8?q?file=20system"=20(putClaudeLogin's=20MkdirAll=20of=20that=20rel?= =?UTF-8?q?ative=20name=20under=20the=20GUI's=20cwd=20/),=20and=20the=20ac?= =?UTF-8?q?count=20lapsed=20minutes=20later=20as=20"Claude=20Code=20is=20n?= =?UTF-8?q?o=20longer=20signed=20in",=20its=20sign-in=20looked=20for=20und?= =?UTF-8?q?er=20that=20folder;=20Codex,=20Pi,=20omp=20and=20the=20rest=20w?= =?UTF-8?q?ere=20pointed=20at=20"$CODEX=5FHOME",=20"$PI=5FCODING=5FAGENT?= =?UTF-8?q?=5FDIR",=20"$OMP=5FPROFILE"=20the=20same=20way,=20in=20the=20GU?= =?UTF-8?q?I=20alone,=20which=20is=20the=20one=20caller=20of=20UserPath)?= =?UTF-8?q?=20=E2=80=94=20shellProbe=20builds=20the=20command:=20for=20a?= =?UTF-8?q?=20shell=20named=20nu=20it=20is=20^/bin/sh=20-c=20''=20(nushell=20has=20no=20POSIX=20quoting=20to=20ask?= =?UTF-8?q?=20with=20and=20its=20PATH=20is=20a=20list,=20while=20a=20child?= =?UTF-8?q?=20of=20it=20gets=20the=20environment=20as=20any=20child=20does?= =?UTF-8?q?,=20PATH=20joined=20back=20into=20one=20string,=20and=20sh=20ex?= =?UTF-8?q?pands=20the=20references;=20the=20outer=20string=20in=20nushell?= =?UTF-8?q?'s=20single=20quotes=20where=20a=20backslash=20is=20a=20backsla?= =?UTF-8?q?sh,=20the=20format=20in=20sh's=20double=20quotes=20where=20\0?= =?UTF-8?q?=20reaches=20printf),=20any=20other=20shell=20keeps=20the=20pri?= =?UTF-8?q?ntf=20it=20had;=20and=20parseShellEnv=20takes=20a=20value=20tha?= =?UTF-8?q?t=20is=20the=20variable's=20own=20reference=20("$CODEX=5FHOME",?= =?UTF-8?q?=20or=20"$PATH"=20for=20PATH)=20for=20unset,=20so=20a=20login?= =?UTF-8?q?=20shell=20with=20other=20quoting=20than=20POSIX=20leaves=20a?= =?UTF-8?q?=20variable=20alone=20rather=20than=20setting=20it=20to=20its?= =?UTF-8?q?=20name.=20TestShellEnvNushell=20runs=20the=20probe=20through?= =?UTF-8?q?=20a=20nushell=20on=20the=20machine=20(skipped=20where=20there?= =?UTF-8?q?=20is=20none)=20and=20checks=20the=20command's=20shape=20anyway?= =?UTF-8?q?;=20TestParseShellEnvUnexpanded=20feeds=20the=20unexpanded=20an?= =?UTF-8?q?swer.=20Verified=20against=20nushell=200.116.0:=20PATH=20comes?= =?UTF-8?q?=20back,=20CODEX=5FHOME=20from=20the=20environment=20reaches=20?= =?UTF-8?q?sh,=20unset=20variables=20are=20"".=20(#739)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: ylorn --- internal/proc/path_other.go | 54 +++++++++++++++++++++++------ internal/proc/path_other_test.go | 59 ++++++++++++++++++++++++++++++++ 2 files changed, 102 insertions(+), 11 deletions(-) diff --git a/internal/proc/path_other.go b/internal/proc/path_other.go index c02e98be..d00799bd 100644 --- a/internal/proc/path_other.go +++ b/internal/proc/path_other.go @@ -166,25 +166,51 @@ func shellEnv() (string, map[string]string) { } ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) defer cancel() - // interactive too, since many put PATH in .zshrc/.bashrc; the marker - // tells the answer apart from whatever the profile prints, and a NUL - // (which no variable can hold) parts the values: PATH first, then each - // of agentenv.Vars in order - const mark = "__magpie_path__" - format, args := mark+"%s", ` "$PATH"` + // interactive too, since many put PATH in .zshrc/.bashrc + cmd := CommandContext(ctx, sh, "-ilc", shellProbe(sh)) + cmd.Stdin = nil + out, _ := cmd.Output() + return parseShellEnv(string(out), shellMark) +} + +// shellMark tells shellEnv's answer apart from whatever the profile prints. +const shellMark = "__magpie_path__" + +// shellProbe is the command the login shell sh runs to print its PATH and +// agentenv.Vars, between two marks. A NUL (which no variable can hold) +// parts the values: PATH first, then each of agentenv.Vars in order. +// +// A POSIX shell (zsh, bash, fish too) expands "$PATH" itself. nushell +// doesn't: a double-quoted string is literal there, so it printed "$PATH", +// "$CLAUDE_CONFIG_DIR", ... and magpie set each variable to its own name +// (#738: then Claude sign-in failed with "mkdir $CLAUDE_CONFIG_DIR: +// read-only file system", and the account lapsed minutes later, Claude +// Code's sign-in looked for under that folder). nushell has no POSIX +// quoting to ask with, and its PATH is a list, so it is asked to run +// /bin/sh with the same printf: nushell hands it the environment as a +// child sees it, PATH joined back into one string, and sh expands the +// references. The outer command is in nushell's single quotes, where a +// backslash is a backslash, and the printf format in sh's double quotes, +// where \0 reaches printf as it is. +func shellProbe(sh string) string { + format, args := shellMark+"%s", ` "$PATH"` for _, v := range agentenv.Vars { format += `\0%s` args += ` "$` + v + `"` } - cmd := CommandContext(ctx, sh, "-ilc", "printf '"+format+mark+"'"+args) - cmd.Stdin = nil - out, _ := cmd.Output() - return parseShellEnv(string(out), mark) + if filepath.Base(sh) == "nu" { + return `^/bin/sh -c 'printf "` + format + shellMark + `"` + args + `'` + } + return "printf '" + format + shellMark + "'" + args } // parseShellEnv reads shellEnv's answer out of what the shell printed. The // variables are nil when their count isn't agentenv.Vars' (a printf that -// doesn't know \0); PATH alone is still taken then. +// doesn't know \0); PATH alone is still taken then. A value that is the +// variable's own reference ("$CODEX_HOME") is a shell that printed the +// text instead of expanding it (nushell before shellProbe knew it; a shell +// with other quoting still), not a folder: it counts as unset, as does a +// PATH of "$PATH". func parseShellEnv(s, mark string) (string, map[string]string) { i := strings.Index(s, mark) if i < 0 { @@ -196,6 +222,9 @@ func parseShellEnv(s, mark string) (string, map[string]string) { return "", nil } f := strings.Split(s[:j], "\x00") + if f[0] == "$PATH" { + f[0] = "" + } if len(f) != len(agentenv.Vars)+1 { if len(f) == 1 { return f[0], nil @@ -204,6 +233,9 @@ func parseShellEnv(s, mark string) (string, map[string]string) { } vars := make(map[string]string, len(agentenv.Vars)) for k, v := range agentenv.Vars { + if f[k+1] == "$"+v { + f[k+1] = "" + } vars[v] = f[k+1] } return f[0], vars diff --git a/internal/proc/path_other_test.go b/internal/proc/path_other_test.go index f72b84ea..7ad0952a 100644 --- a/internal/proc/path_other_test.go +++ b/internal/proc/path_other_test.go @@ -9,6 +9,8 @@ import ( "strings" "testing" "time" + + "github.com/yetone/magpie/internal/agentenv" ) // A desktop app with launchd's PATH finds a claude installed under a custom @@ -82,3 +84,60 @@ func TestUserPathTakesAgentVars(t *testing.T) { t.Fatalf("PATH lost: %q", os.Getenv("PATH")) } } + +// A login shell that doesn't expand "$CLAUDE_CONFIG_DIR" prints the +// reference itself. That is no folder: the variable counts as unset, +// rather than magpie setting CLAUDE_CONFIG_DIR to "$CLAUDE_CONFIG_DIR" and +// then looking for Claude Code's sign-in under it (#738). +func TestParseShellEnvUnexpanded(t *testing.T) { + s := shellMark + "$PATH" + for _, v := range agentenv.Vars { + s += "\x00$" + v + } + p, vars := parseShellEnv("hello from the profile\n"+s+shellMark, shellMark) + if p != "" { + t.Fatalf("PATH = %q, want none", p) + } + if len(vars) != len(agentenv.Vars) { + t.Fatalf("%d variables, want %d", len(vars), len(agentenv.Vars)) + } + for v, got := range vars { + if got != "" { + t.Fatalf("%s = %q, want none", v, got) + } + } +} + +// nushell is asked in its own way, and answers with its PATH and the +// variables a child of it gets, as a POSIX shell does; where nushell isn't +// installed, the command's shape is all that can be checked. +func TestShellEnvNushell(t *testing.T) { + probe := shellProbe("/opt/homebrew/bin/nu") + if !strings.HasPrefix(probe, "^/bin/sh -c 'printf \""+shellMark+"%s") || !strings.HasSuffix(probe, `"$`+agentenv.Vars[len(agentenv.Vars)-1]+`"'`) { + t.Fatalf("nushell's probe: %q", probe) + } + if strings.Contains(probe, `'`+shellMark) || strings.Count(probe, "'") != 2 { + t.Fatalf("nushell's probe must hold sh's command in one pair of single quotes: %q", probe) + } + nu, err := exec.LookPath("nu") + if err != nil { + t.Skip("nushell isn't installed") + } + t.Setenv("CODEX_HOME", "/from/the/environment") + t.Setenv("CLAUDE_CONFIG_DIR", "") + os.Unsetenv("CLAUDE_CONFIG_DIR") + out, err := exec.Command(nu, "-ilc", shellProbe(nu)).Output() + if err != nil { + t.Fatalf("nu: %v", err) + } + p, vars := parseShellEnv(string(out), shellMark) + if p == "" || strings.Contains(p, "$PATH") { + t.Fatalf("PATH from nushell: %q", p) + } + if got := vars["CODEX_HOME"]; got != "/from/the/environment" { + t.Fatalf("CODEX_HOME = %q: nushell's environment didn't reach sh", got) + } + if got := vars["CLAUDE_CONFIG_DIR"]; got != "" { + t.Fatalf("CLAUDE_CONFIG_DIR = %q, want none", got) + } +}