diff --git a/cmd/gortex/daemon_service.go b/cmd/gortex/daemon_service.go index b3159dacc..1e935b038 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,30 @@ 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. 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) && !strings.ContainsAny(entry, "\r\n") && !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. @@ -105,17 +130,20 @@ 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 -// common case) pass through unchanged. +// silently change the value the daemon sees. An assignment containing +// 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(`\`, `\\`, `"`, `\"`) @@ -200,10 +228,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 +257,7 @@ const launchdPlistTemplate = ` EnvironmentVariables PATH - /usr/local/bin:/opt/homebrew/bin:/usr/bin:/bin + {{.Path}} {{- range .EnvVars}} {{.Key}} {{.Value}} @@ -245,12 +272,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 +592,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 @@ -576,7 +605,7 @@ After=network.target Type=simple ExecStart={{.Exe}} daemon start {{- range .EnvVars}} -Environment={{.Key}}={{.Value}} +Environment={{.}} {{- end}} Restart=on-failure RestartSec=2 @@ -588,14 +617,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 3819f3209..e7b904035 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", @@ -127,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]") @@ -163,11 +166,132 @@ 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"}, + { + 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) + 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, "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, "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")) } +// 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.