From ab27f7c1238f7804dd11d365fe10abbbac9c2f3c Mon Sep 17 00:00:00 2001 From: Peter Chanthamynavong Date: Fri, 2 Oct 2026 13:24:34 -0700 Subject: [PATCH 1/4] fix(daemon): capture the installing shell's PATH in the service unit The launchd plist hard-coded PATH to /usr/local/bin:/opt/homebrew/bin:/usr/bin:/bin and the systemd unit set none, so language servers installed elsewhere (for example /usr/local/share/dotnet or ~/go/bin) were not found by the supervised daemon, and a hand-edited PATH was lost on the next install-service. install-service now captures the absolute entries of the installing shell's PATH, without duplicates and in the shell's order, like xdgServiceEnv does for XDG_*. launchd appends any missing old defaults, so an empty PATH renders the old value. systemd gets Environment=PATH only when something was captured. No other variables are captured. --- cmd/gortex/daemon_service.go | 47 ++++++++++--- cmd/gortex/daemon_service_test.go | 109 ++++++++++++++++++++++++++++++ 2 files changed, 146 insertions(+), 10 deletions(-) diff --git a/cmd/gortex/daemon_service.go b/cmd/gortex/daemon_service.go index b3159dacc..3eb72f94b 100644 --- a/cmd/gortex/daemon_service.go +++ b/cmd/gortex/daemon_service.go @@ -10,6 +10,7 @@ import ( "os/exec" "path/filepath" "runtime" + "slices" "strings" "text/template" @@ -94,6 +95,28 @@ func xdgServiceEnv() []serviceEnvVar { return out } +// servicePath captures the installing shell's PATH so supervised language +// servers can be found outside the standard locations. Re-run install-service +// to re-capture changed values. Only absolute entries are kept: relative and +// empty entries would resolve against the service's working directory. Entries +// need not exist yet; duplicates are removed without changing the shell's order. +// Missing defaults are appended for launchd; systemd keeps its default when the +// captured PATH is empty. +func servicePath(defaults []string) string { + var entries []string + for _, entry := range strings.Split(os.Getenv("PATH"), string(os.PathListSeparator)) { + if filepath.IsAbs(entry) && !slices.Contains(entries, entry) { + entries = append(entries, entry) + } + } + for _, entry := range defaults { + if !slices.Contains(entries, entry) { + entries = append(entries, entry) + } + } + return strings.Join(entries, string(os.PathListSeparator)) +} + // xmlEscape renders s safe for an XML text node (the launchd plist) so a // home path containing an XML metacharacter can't produce a malformed, // unloadable plist. @@ -200,10 +223,9 @@ func runDaemonServiceStatus(cmd *cobra.Command, _ []string) error { // StandardOutPath / StandardErrorPath redirect logs into the same file // `gortex daemon logs` tails, so users don't need to remember two paths. // -// EnvironmentVariables carries PATH (so a Homebrew-installed binary is -// found in launchd's minimal environment) plus any XDG_* overrides that -// were in effect at install time — see xdgServiceEnv for why that -// capture is necessary. +// EnvironmentVariables carries the installing shell's PATH with missing +// Homebrew / system defaults appended, plus any XDG_* overrides in effect at +// install time — see servicePath and xdgServiceEnv for the capture rules. const launchdPlistTemplate = ` @@ -230,7 +252,7 @@ const launchdPlistTemplate = ` EnvironmentVariables PATH - /usr/local/bin:/opt/homebrew/bin:/usr/bin:/bin + {{.Path}} {{- range .EnvVars}} {{.Key}} {{.Value}} @@ -245,12 +267,13 @@ const launchdPlistTemplate = ` // malformed, unloadable plist. func renderLaunchdPlist(label, exe, logPath string, env []serviceEnvVar) (string, error) { data := struct { - Label, Exe, LogPath string - EnvVars []serviceEnvVar + Label, Exe, LogPath, Path string + EnvVars []serviceEnvVar }{ Label: xmlEscape(label), Exe: xmlEscape(exe), LogPath: xmlEscape(logPath), + Path: xmlEscape(servicePath([]string{"/usr/local/bin", "/opt/homebrew/bin", "/usr/bin", "/bin"})), EnvVars: make([]serviceEnvVar, len(env)), } for i, e := range env { @@ -564,9 +587,10 @@ func noteRunningDaemon(w io.Writer, daemonRunning func() bool) { // systemdUnitTemplate renders a user-level systemd service. Type=simple // because `gortex daemon start` (without --detach) runs in the // foreground; Restart=on-failure covers the crash-restart case without -// pounding on successful exits. Environment= lines carry any XDG_* -// overrides that were in effect at install time so the supervised daemon -// resolves the same paths as the installing shell — see xdgServiceEnv. +// pounding on successful exits. Environment= lines carry a non-empty captured +// PATH and any XDG_* overrides in effect at install time so the supervised +// daemon resolves the same paths as the installing shell — see servicePath +// and xdgServiceEnv. const systemdUnitTemplate = `[Unit] Description=Gortex code intelligence daemon Documentation=https://github.com/zzet/gortex @@ -590,6 +614,9 @@ WantedBy=default.target // renderSystemdUnit fills systemdUnitTemplate, quoting Environment= // values that need it. func renderSystemdUnit(exe, logPath string, env []serviceEnvVar) (string, error) { + if path := servicePath(nil); path != "" { + env = append([]serviceEnvVar{{Key: "PATH", Value: path}}, env...) + } data := struct { Exe, LogPath string EnvVars []serviceEnvVar diff --git a/cmd/gortex/daemon_service_test.go b/cmd/gortex/daemon_service_test.go index 3819f3209..6a0e56650 100644 --- a/cmd/gortex/daemon_service_test.go +++ b/cmd/gortex/daemon_service_test.go @@ -3,6 +3,7 @@ package main import ( "encoding/xml" "io" + "os" "path/filepath" "runtime" "strings" @@ -96,6 +97,7 @@ func TestRenderLaunchdPlist_EscapesXML(t *testing.T) { // contract and that no Environment= line is emitted when nothing was // captured. func TestRenderSystemdUnit_NoXDG(t *testing.T) { + t.Setenv("PATH", "") out, err := renderSystemdUnit( "/home/u/.local/bin/gortex", "/home/u/.gortex/cache/daemon.log", @@ -163,6 +165,113 @@ func TestXDGServiceEnv_OnlyAbsoluteSet(t *testing.T) { assert.False(t, hasCache, "empty XDG_CACHE_HOME must be ignored") } +func TestServicePath(t *testing.T) { + sep := string(os.PathListSeparator) + root := t.TempDir() + first := filepath.Join(root, "go", "bin") + second := filepath.Join(root, "dotnet") + require.NoDirExists(t, first, "PATH entries need not exist at install time") + defaults := []string{"/usr/local/bin", "/opt/homebrew/bin", "/usr/bin", "/bin"} + + for _, tt := range []struct { + name string + path string + defaults []string + want string + }{ + { + name: "absolute entries only, first occurrence and order kept", + path: strings.Join([]string{"", ".", second, "relative/bin", first, second, ""}, sep), + want: second + sep + first, + }, + { + name: "launchd appends missing defaults without changing precedence", + path: strings.Join([]string{second, first, second}, sep), + defaults: []string{first, second, filepath.Join(root, "default")}, + want: strings.Join([]string{second, first, filepath.Join(root, "default")}, sep), + }, + { + name: "systemd has no appended defaults", + path: first, + want: first, + }, + { + name: "empty launchd PATH preserves the old defaults", + defaults: defaults, + want: strings.Join(defaults, sep), + }, + {name: "empty systemd PATH stays empty"}, + {name: "all relative systemd PATH stays empty", path: sep + "." + sep + "relative/bin"}, + } { + t.Run(tt.name, func(t *testing.T) { + t.Setenv("PATH", tt.path) + assert.Equal(t, tt.want, servicePath(tt.defaults)) + }) + } +} + +func TestRenderLaunchdPlist_CapturesPATH(t *testing.T) { + sep := string(os.PathListSeparator) + custom := filepath.Join(t.TempDir(), "a&b", "100% tools") + t.Setenv("PATH", custom) + t.Setenv("GORTEX_DAEMON_HTTP_TOKEN", "must-not-be-captured") + out, err := renderLaunchdPlist("com.zzet.gortex", "/bin/gortex", "/log", nil) + require.NoError(t, err) + + want := strings.ReplaceAll(custom, "&", "&") + sep + + strings.Join([]string{"/usr/local/bin", "/opt/homebrew/bin", "/usr/bin", "/bin"}, sep) + assert.Contains(t, out, "PATH\n "+want+"") + assert.NotContains(t, out, "a&b") + assert.NotContains(t, out, "GORTEX_") + assert.NotContains(t, out, "must-not-be-captured") + assertWellFormedXML(t, out) +} + +func TestRenderLaunchdPlist_EmptyPATH(t *testing.T) { + t.Setenv("PATH", "") + out, err := renderLaunchdPlist("com.zzet.gortex", "/bin/gortex", "/log", nil) + require.NoError(t, err) + + // This is exactly the old PATH on launchd's Unix host; adapt only the + // path-list separator when these render tests run on Windows. + want := strings.ReplaceAll("/usr/local/bin:/opt/homebrew/bin:/usr/bin:/bin", ":", string(os.PathListSeparator)) + assert.Contains(t, out, "PATH\n "+want+"") + assert.Equal(t, 1, strings.Count(out, "PATH")) + assertWellFormedXML(t, out) +} + +func TestRenderSystemdUnit_CapturesPATH(t *testing.T) { + sep := string(os.PathListSeparator) + root := t.TempDir() + first := filepath.Join(root, "a&b", "100% tools") + second := filepath.Join(root, "bin") + t.Setenv("PATH", first+sep+second) + t.Setenv("GORTEX_DAEMON_HTTP_TOKEN", "must-not-be-captured") + out, err := renderSystemdUnit("/bin/gortex", "/log", nil) + require.NoError(t, err) + + // systemd quotes whitespace and escapes backslashes within a quoted + // value (relevant to this native fixture when tests run on Windows). + want := strings.ReplaceAll(first+sep+second, "\\", "\\\\") + want = strings.ReplaceAll(want, "%", "%%") + assert.Contains(t, out, "\nEnvironment=PATH=\""+want+"\"\n") + assert.Equal(t, 1, strings.Count(out, "Environment=PATH=")) + assert.NotContains(t, out, "/opt/homebrew/bin") + assert.NotContains(t, out, "GORTEX_") + assert.NotContains(t, out, "must-not-be-captured") +} + +func TestRenderSystemdUnit_EmptyPATH(t *testing.T) { + for _, path := range []string{"", ".", string(os.PathListSeparator) + "relative/bin"} { + t.Run(path, func(t *testing.T) { + t.Setenv("PATH", path) + out, err := renderSystemdUnit("/bin/gortex", "/log", nil) + require.NoError(t, err) + assert.NotContains(t, out, "Environment=PATH=") + }) + } +} + func TestSystemdEnvValue_QuotesWhitespace(t *testing.T) { assert.Equal(t, "/home/u/.config", systemdEnvValue("/home/u/.config")) assert.Equal(t, `"/home/u/my data"`, systemdEnvValue("/home/u/my data")) From 939e8722dbfb5201e36449a08991d1e218d6a752 Mon Sep 17 00:00:00 2001 From: Peter Chanthamynavong Date: Fri, 2 Oct 2026 15:24:16 -0700 Subject: [PATCH 2/4] fix(daemon): drop PATH entries that contain CR or LF A captured PATH entry with a line break would split the systemd unit's Environment= line into a separate directive. Such an entry cannot name a usable directory, so servicePath now skips it like a relative entry. --- cmd/gortex/daemon_service.go | 8 +++++--- cmd/gortex/daemon_service_test.go | 5 +++++ 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/cmd/gortex/daemon_service.go b/cmd/gortex/daemon_service.go index 3eb72f94b..ff1c76c0f 100644 --- a/cmd/gortex/daemon_service.go +++ b/cmd/gortex/daemon_service.go @@ -98,14 +98,16 @@ func xdgServiceEnv() []serviceEnvVar { // servicePath captures the installing shell's PATH so supervised language // servers can be found outside the standard locations. Re-run install-service // to re-capture changed values. Only absolute entries are kept: relative and -// empty entries would resolve against the service's working directory. Entries -// need not exist yet; duplicates are removed without changing the shell's order. +// empty entries would resolve against the service's working directory. An +// entry containing CR or LF is dropped, because it would split a systemd +// Environment= line. Entries need not exist yet; duplicates are removed without +// changing the shell's order. // Missing defaults are appended for launchd; systemd keeps its default when the // captured PATH is empty. func servicePath(defaults []string) string { var entries []string for _, entry := range strings.Split(os.Getenv("PATH"), string(os.PathListSeparator)) { - if filepath.IsAbs(entry) && !slices.Contains(entries, entry) { + if filepath.IsAbs(entry) && !strings.ContainsAny(entry, "\r\n") && !slices.Contains(entries, entry) { entries = append(entries, entry) } } diff --git a/cmd/gortex/daemon_service_test.go b/cmd/gortex/daemon_service_test.go index 6a0e56650..54af0d271 100644 --- a/cmd/gortex/daemon_service_test.go +++ b/cmd/gortex/daemon_service_test.go @@ -202,6 +202,11 @@ func TestServicePath(t *testing.T) { }, {name: "empty systemd PATH stays empty"}, {name: "all relative systemd PATH stays empty", path: sep + "." + sep + "relative/bin"}, + { + name: "entries containing CR or LF are dropped", + path: strings.Join([]string{first + "\nExecStartPre=/bin/false", second + "\r", first}, sep), + want: first, + }, } { t.Run(tt.name, func(t *testing.T) { t.Setenv("PATH", tt.path) From b42eb3fe8af16b1a899d13b3a7fafeca46bae019 Mon Sep 17 00:00:00 2001 From: Peter Chanthamynavong Date: Fri, 2 Oct 2026 15:25:53 -0700 Subject: [PATCH 3/4] fix(daemon): quote the whole systemd Environment= assignment systemd.syntax(7) recognizes a quote only at the start of an item, so Environment=KEY="a b" does not unquote the value. When a captured PATH or XDG_* value contains whitespace, quote the complete KEY=value assignment instead, as systemd.exec(5) shows: Environment="KEY=a b". --- cmd/gortex/daemon_service.go | 20 +++++++++++--------- cmd/gortex/daemon_service_test.go | 11 ++++++----- 2 files changed, 17 insertions(+), 14 deletions(-) diff --git a/cmd/gortex/daemon_service.go b/cmd/gortex/daemon_service.go index ff1c76c0f..3e12809c7 100644 --- a/cmd/gortex/daemon_service.go +++ b/cmd/gortex/daemon_service.go @@ -130,13 +130,15 @@ func xmlEscape(s string) string { return b.String() } -// systemdEnvValue renders a value safe for a systemd Environment= line. +// systemdEnvValue renders a KEY=value assignment safe for a systemd +// Environment= line. // `%` is escaped to `%%` because systemd treats it as a specifier // introducer across the whole unit file (systemd.unit(5)) — an // unescaped `%d` in a path would expand to a directory specifier and -// silently change the value the daemon sees. Values containing -// whitespace are additionally double-quoted (with embedded quotes / -// backslashes escaped) per systemd's quoting rules. Plain paths (the +// silently change the value the daemon sees. An assignment containing +// whitespace is additionally double-quoted as a whole (with embedded quotes / +// backslashes escaped): systemd.syntax(7) recognizes a quote only at the +// start of an item, so `KEY="a b"` would not be unquoted. Plain paths (the // common case) pass through unchanged. func systemdEnvValue(v string) string { v = strings.ReplaceAll(v, "%", "%%") @@ -602,7 +604,7 @@ After=network.target Type=simple ExecStart={{.Exe}} daemon start {{- range .EnvVars}} -Environment={{.Key}}={{.Value}} +Environment={{.}} {{- end}} Restart=on-failure RestartSec=2 @@ -614,17 +616,17 @@ WantedBy=default.target ` // renderSystemdUnit fills systemdUnitTemplate, quoting Environment= -// values that need it. +// assignments that need it. func renderSystemdUnit(exe, logPath string, env []serviceEnvVar) (string, error) { if path := servicePath(nil); path != "" { env = append([]serviceEnvVar{{Key: "PATH", Value: path}}, env...) } data := struct { Exe, LogPath string - EnvVars []serviceEnvVar - }{Exe: exe, LogPath: logPath, EnvVars: make([]serviceEnvVar, len(env))} + EnvVars []string + }{Exe: exe, LogPath: logPath, EnvVars: make([]string, len(env))} for i, e := range env { - data.EnvVars[i] = serviceEnvVar{Key: e.Key, Value: systemdEnvValue(e.Value)} + data.EnvVars[i] = systemdEnvValue(e.Key + "=" + e.Value) } var buf bytes.Buffer if err := template.Must(template.New("unit").Parse(systemdUnitTemplate)).Execute(&buf, data); err != nil { diff --git a/cmd/gortex/daemon_service_test.go b/cmd/gortex/daemon_service_test.go index 54af0d271..338738eef 100644 --- a/cmd/gortex/daemon_service_test.go +++ b/cmd/gortex/daemon_service_test.go @@ -129,8 +129,9 @@ func TestRenderSystemdUnit_PropagatesXDG(t *testing.T) { require.NoError(t, err) assert.Contains(t, out, "Environment=XDG_CACHE_HOME=/home/u/.cache") - // A value containing whitespace is double-quoted per systemd rules. - assert.Contains(t, out, `Environment=XDG_DATA_HOME="/home/u/has space"`) + // An assignment containing whitespace is double-quoted as a whole, + // because systemd recognizes a quote only at the start of an item. + assert.Contains(t, out, `Environment="XDG_DATA_HOME=/home/u/has space"`) // Environment lines must sit inside [Service], ahead of [Install]. svcStart := strings.Index(out, "[Service]") @@ -259,8 +260,8 @@ func TestRenderSystemdUnit_CapturesPATH(t *testing.T) { // value (relevant to this native fixture when tests run on Windows). want := strings.ReplaceAll(first+sep+second, "\\", "\\\\") want = strings.ReplaceAll(want, "%", "%%") - assert.Contains(t, out, "\nEnvironment=PATH=\""+want+"\"\n") - assert.Equal(t, 1, strings.Count(out, "Environment=PATH=")) + assert.Contains(t, out, "\nEnvironment=\"PATH="+want+"\"\n") + assert.Equal(t, 1, strings.Count(out, "PATH=")) assert.NotContains(t, out, "/opt/homebrew/bin") assert.NotContains(t, out, "GORTEX_") assert.NotContains(t, out, "must-not-be-captured") @@ -272,7 +273,7 @@ func TestRenderSystemdUnit_EmptyPATH(t *testing.T) { t.Setenv("PATH", path) out, err := renderSystemdUnit("/bin/gortex", "/log", nil) require.NoError(t, err) - assert.NotContains(t, out, "Environment=PATH=") + assert.NotContains(t, out, "PATH=") }) } } From db6bd6e5325adb114aea5564af0481ba92bc521b Mon Sep 17 00:00:00 2001 From: Peter Chanthamynavong Date: Fri, 2 Oct 2026 15:43:42 -0700 Subject: [PATCH 4/4] fix(daemon): quote systemd assignments that contain a backslash or quote systemd applies C-style escapes to unquoted Environment= text, so a PATH entry such as /opt/a\tools would reach the daemon with a tab in it. An unquoted quote character would start a quoted section. systemdEnvValue now also quotes the whole assignment when it contains a backslash or a quote, and escapes backslashes and double quotes inside the quotes. --- cmd/gortex/daemon_service.go | 11 ++++++----- cmd/gortex/daemon_service_test.go | 9 +++++++++ 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/cmd/gortex/daemon_service.go b/cmd/gortex/daemon_service.go index 3e12809c7..1e935b038 100644 --- a/cmd/gortex/daemon_service.go +++ b/cmd/gortex/daemon_service.go @@ -136,13 +136,14 @@ func xmlEscape(s string) string { // introducer across the whole unit file (systemd.unit(5)) — an // unescaped `%d` in a path would expand to a directory specifier and // silently change the value the daemon sees. An assignment containing -// whitespace is additionally double-quoted as a whole (with embedded quotes / -// backslashes escaped): systemd.syntax(7) recognizes a quote only at the -// start of an item, so `KEY="a b"` would not be unquoted. Plain paths (the -// common case) pass through unchanged. +// whitespace, a backslash or a quote is additionally double-quoted as a whole +// (with embedded quotes / backslashes escaped): systemd.syntax(7) recognizes +// a quote only at the start of an item, so `KEY="a b"` would not be unquoted, +// and systemd applies C-style escapes such as `\t` to unquoted text too. +// Plain paths (the common case) pass through unchanged. func systemdEnvValue(v string) string { v = strings.ReplaceAll(v, "%", "%%") - if !strings.ContainsAny(v, " \t") { + if !strings.ContainsAny(v, " \t\\\"'") { return v } r := strings.NewReplacer(`\`, `\\`, `"`, `\"`) diff --git a/cmd/gortex/daemon_service_test.go b/cmd/gortex/daemon_service_test.go index 338738eef..e7b904035 100644 --- a/cmd/gortex/daemon_service_test.go +++ b/cmd/gortex/daemon_service_test.go @@ -283,6 +283,15 @@ func TestSystemdEnvValue_QuotesWhitespace(t *testing.T) { assert.Equal(t, `"/home/u/my data"`, systemdEnvValue("/home/u/my data")) } +// TestSystemdEnvValue_QuotesEscapesAndQuotes covers characters systemd +// interprets in unquoted text: a backslash starts a C-style escape (`\t` +// would become a tab), and a quote starts a quoted section. +func TestSystemdEnvValue_QuotesEscapesAndQuotes(t *testing.T) { + assert.Equal(t, `"PATH=/opt/a\\tools"`, systemdEnvValue(`PATH=/opt/a\tools`)) + assert.Equal(t, `"PATH=/opt/\"q\"/bin"`, systemdEnvValue(`PATH=/opt/"q"/bin`)) + assert.Equal(t, `"PATH=/opt/it's/bin"`, systemdEnvValue(`PATH=/opt/it's/bin`)) +} + // TestSystemdEnvValue_EscapesPercent guards the systemd specifier escape: // a literal % in a path must become %% or systemd expands it (e.g. %d) // and the daemon resolves a different directory than was captured.