diff --git a/config.example.toml b/config.example.toml index b7706d1e0..d1e604f66 100644 --- a/config.example.toml +++ b/config.example.toml @@ -37,6 +37,10 @@ # Session data storage directory (default: ~/.cc-connect) # Stores conversation history and session state as JSON files. # 会话数据存储目录（默认 ~/.cc-connect），以 JSON 文件保存对话历史和会话状态。 +# NOTE: Prefer an absolute path when running under systemd / launchd — service +# managers may not propagate HOME, and any relative value would be resolved +# against work_dir, which agent subprocesses cd into before reading files. +# 建议在 systemd/launchd 部署下使用绝对路径，避免 HOME 缺失时相对路径解析到 work_dir。 # data_dir = "/path/to/custom/dir" # Attachment send-back switch for `cc-connect send --image/--file`. diff --git a/config/config.go b/config/config.go index 908027db3..2b69db0fb 100644 --- a/config/config.go +++ b/config/config.go @@ -631,9 +631,24 @@ func load(path string) (*Config, error) { if home, err := os.UserHomeDir(); err == nil { cfg.DataDir = filepath.Join(home, ".cc-connect") } else { + // HOME unset (common when systemd unit does not inject it): + // warn loudly, because a relative data_dir will be resolved + // differently by any subprocess that changes cwd — most + // notably the claude/codex/etc. agent processes that cd into + // work_dir before reading files written by the supervisor. cfg.DataDir = ".cc-connect" + slog.Warn("config: data_dir empty and HOME unset; falling back to relative path. Set data_dir to an absolute path in config.toml, or ensure the service manager injects HOME.") } } + cfg.DataDir = expandUserPath(cfg.DataDir) + // Always resolve to an absolute path so a data_dir written by the + // supervisor is read back at the same location by agent subprocesses + // regardless of their working directory. Users who write a relative + // data_dir explicitly still get it anchored to the supervisor's cwd + // (the same behaviour they would see running any other CLI). + if abs, err := filepath.Abs(cfg.DataDir); err == nil { + cfg.DataDir = abs + } cfg.AttachmentSend = strings.ToLower(strings.TrimSpace(cfg.AttachmentSend)) if cfg.AttachmentSend == "" { cfg.AttachmentSend = "on" diff --git a/config/config_test.go b/config/config_test.go index 505067234..f5e02971e 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -583,6 +583,84 @@ func TestLoad_DefaultsDataDir(t *testing.T) { } } +// TestLoad_DataDirAbsoluteWhenExplicitlyRelative regresses the +// "Append system prompt file not found" bug: even when a user sets +// data_dir to a relative path in config.toml, Load must anchor it to +// an absolute path so that agent subprocesses (which cd into +// project.work_dir before reading system prompt files) do not resolve +// data_dir against the child's working directory instead of the +// supervisor's. +func TestLoad_DataDirAbsoluteWhenExplicitlyRelative(t *testing.T) { + dir := t.TempDir() + t.Setenv("HOME", dir) + + cfgPath := filepath.Join(dir, "config.toml") + body := "data_dir = \"./my-data\"\n" + baseConfigTOML + if err := os.WriteFile(cfgPath, []byte(body), 0o644); err != nil { + t.Fatalf("write config: %v", err) + } + + cfg, err := Load(cfgPath) + if err != nil { + t.Fatalf("Load() error: %v", err) + } + if !filepath.IsAbs(cfg.DataDir) { + t.Fatalf("Load() data_dir = %q, want absolute path", cfg.DataDir) + } + if !strings.HasSuffix(cfg.DataDir, "my-data") { + t.Fatalf("Load() data_dir = %q, want suffix my-data", cfg.DataDir) + } +} + +func TestLoad_DataDirExpandsCurrentUserHome(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + + cfgPath := filepath.Join(t.TempDir(), "config.toml") + body := "data_dir = \"~/.cc-connect\"\n" + baseConfigTOML + if err := os.WriteFile(cfgPath, []byte(body), 0o644); err != nil { + t.Fatalf("write config: %v", err) + } + + cfg, err := Load(cfgPath) + if err != nil { + t.Fatalf("Load() error: %v", err) + } + + want := filepath.Join(home, ".cc-connect") + if cfg.DataDir != want { + t.Fatalf("Load() data_dir = %q, want %q", cfg.DataDir, want) + } +} + +// TestLoad_DataDirAbsoluteWhenHOMEEmpty covers the fallback path when +// os.UserHomeDir fails. This is exercised by service managers like +// launchd (gui/... bootstrap) and systemd (system units) that do not +// propagate HOME. Even in that degraded state, Load must still return +// an absolute data_dir so subprocesses do not silently write to +// /.cc-connect. Windows uses USERPROFILE, which we cannot +// reliably clear from a test. +func TestLoad_DataDirAbsoluteWhenHOMEEmpty(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("HOME does not gate os.UserHomeDir on windows") + } + dir := t.TempDir() + cfgPath := filepath.Join(dir, "config.toml") + if err := os.WriteFile(cfgPath, []byte(baseConfigTOML), 0o644); err != nil { + t.Fatalf("write config: %v", err) + } + + t.Setenv("HOME", "") + + cfg, err := Load(cfgPath) + if err != nil { + t.Fatalf("Load() error: %v", err) + } + if !filepath.IsAbs(cfg.DataDir) { + t.Fatalf("Load() data_dir = %q, want absolute path even when HOME unset", cfg.DataDir) + } +} + func TestLoad_ResolvesEnvPlaceholders(t *testing.T) { root := t.TempDir() diff --git a/daemon/launchd.go b/daemon/launchd.go index c95ed13e0..1ce35b94b 100644 --- a/daemon/launchd.go +++ b/daemon/launchd.go @@ -257,6 +257,7 @@ var templateOwnedEnvKeys = map[string]struct{}{ "CC_LOG_FILE": {}, "CC_LOG_MAX_SIZE": {}, "PATH": {}, + "HOME": {}, } // renderEnvExtraPlist returns the serialized key/value pairs (without the @@ -302,6 +303,17 @@ func buildPlist(cfg Config) string { envPATH = "/usr/local/bin:/usr/bin:/bin:/opt/homebrew/bin" } envExtra := renderEnvExtraPlist(cfg.EnvExtra) + // HOME: launchd LaunchAgents run under the user's uid, but the HOME + // env var is not always populated (especially when bootstrapped via + // `launchctl bootstrap gui/...`). Without HOME, os.UserHomeDir() + // returns an error inside the daemon and any subprocess it spawns; + // config.Load then falls back to a relative data_dir which agents + // (cd'd into work_dir) resolve to /.cc-connect instead of + // /.cc-connect. + homeEntry := "" + if cfg.HomeDir != "" { + homeEntry = fmt.Sprintf("\t\tHOME\n\t\t%s\n", xmlEscape(cfg.HomeDir)) + } // User-supplied paths can legitimately contain XML-special characters // ('&', '<', '>', '"', '\''). Without escaping, `launchctl bootstrap` // rejects the plist with a parse error and daemon install fails. The @@ -341,13 +353,12 @@ func buildPlist(cfg Config) string { %d PATH %s -%s +%s%s StandardOutPath /dev/null StandardErrorPath /dev/null -`, launchdLabel, xmlEscape(cfg.BinaryPath), xmlEscape(cfg.WorkDir), xmlEscape(cfg.LogFile), cfg.LogMaxSize, cfg.LogMaxBackups, xmlEscape(envPATH), envExtra) +`, launchdLabel, xmlEscape(cfg.BinaryPath), xmlEscape(cfg.WorkDir), xmlEscape(cfg.LogFile), cfg.LogMaxSize, cfg.LogMaxBackups, xmlEscape(envPATH), homeEntry, envExtra) } - diff --git a/daemon/launchd_test.go b/daemon/launchd_test.go index abced9953..af3e560ad 100644 --- a/daemon/launchd_test.go +++ b/daemon/launchd_test.go @@ -512,3 +512,61 @@ func TestInstallLaunchd_TightensExistingPlistFrom0644(t *testing.T) { t.Errorf("plist mode after reinstall = %o, want 0600", info.Mode().Perm()) } } + +// TestBuildPlist_IncludesHOME regresses the "Append system prompt file not +// found" bug: launchd LaunchAgents do not always propagate HOME, so the +// daemon (and Claude / Codex / etc. subprocesses it spawns) would call +// os.UserHomeDir(), get an error, and config.Load would fall back to a +// relative data_dir that agent subprocesses resolved against work_dir. +func TestBuildPlist_IncludesHOME(t *testing.T) { + cfg := Config{ + BinaryPath: "/bin/true", + WorkDir: "/tmp/wd", + LogFile: "/tmp/log", + LogMaxSize: 1024, + EnvPATH: "/usr/bin", + HomeDir: "/home/app", + } + out := buildPlist(cfg) + if !strings.Contains(out, "HOME\n\t\t/home/app") { + t.Fatalf("plist should include HOME=/home/app; got:\n%s", out) + } +} + +// TestBuildPlist_OmitsHOMEWhenEmpty ensures we do not emit an empty +// for HOME when the caller could not determine one. +func TestBuildPlist_OmitsHOMEWhenEmpty(t *testing.T) { + cfg := Config{ + BinaryPath: "/bin/true", + WorkDir: "/tmp/wd", + LogFile: "/tmp/log", + LogMaxSize: 1024, + EnvPATH: "/usr/bin", + } + out := buildPlist(cfg) + if strings.Contains(out, "HOME") { + t.Fatalf("plist should omit HOME when HomeDir empty; got:\n%s", out) + } +} + +// TestBuildPlist_EnvExtraHOMEDoesNotOverrideTemplateHOME pins that the +// template-owned HOME wins over any HOME leaking in through EnvExtra — +// the same guarantee we make for PATH and CC_LOG_FILE. +func TestBuildPlist_EnvExtraHOMEDoesNotOverrideTemplateHOME(t *testing.T) { + cfg := Config{ + BinaryPath: "/bin/true", + WorkDir: "/tmp/wd", + LogFile: "/tmp/log", + LogMaxSize: 1024, + EnvPATH: "/usr/bin", + HomeDir: "/home/app", + EnvExtra: map[string]string{"HOME": "/should-not-override"}, + } + out := buildPlist(cfg) + if strings.Contains(out, "/should-not-override") { + t.Fatalf("EnvExtra HOME leaked past template ownership: %s", out) + } + if !strings.Contains(out, "/home/app") { + t.Fatalf("expected template HOME /home/app to survive; got:\n%s", out) + } +} diff --git a/daemon/manager.go b/daemon/manager.go index 0deca7953..02ead8972 100644 --- a/daemon/manager.go +++ b/daemon/manager.go @@ -21,13 +21,14 @@ const ( ) type Config struct { - BinaryPath string - WorkDir string - LogFile string - LogMaxSize int64 - LogMaxBackups int - EnvPATH string // capture user's PATH so agents are accessible - EnvExtra map[string]string // selected environment variables needed by the service runtime + BinaryPath string + WorkDir string + HomeDir string // captured HOME so subprocesses can resolve ~/… paths (issue: relative data_dir fallback) + LogFile string + LogMaxSize int64 + LogMaxBackups int + EnvPATH string // capture user's PATH so agents are accessible + EnvExtra map[string]string // selected environment variables needed by the service runtime // NoCaptureSecrets, when true, restricts the install-time env capture // to proxy-related variables only and skips both the config.toml ${ENV} // placeholder scan and any extension discoverers registered via @@ -74,12 +75,12 @@ func DefaultDataDir() string { // etc. can locate the log file without parsing service definitions. type Meta struct { - LogFile string `json:"log_file"` - LogMaxSize int64 `json:"log_max_size"` - LogMaxBackups int `json:"log_max_backups"` - WorkDir string `json:"work_dir"` - BinaryPath string `json:"binary_path"` - InstalledAt string `json:"installed_at"` + LogFile string `json:"log_file"` + LogMaxSize int64 `json:"log_max_size"` + LogMaxBackups int `json:"log_max_backups"` + WorkDir string `json:"work_dir"` + BinaryPath string `json:"binary_path"` + InstalledAt string `json:"installed_at"` } func metaPath() string { @@ -148,6 +149,15 @@ func Resolve(cfg *Config) error { if cfg.EnvPATH == "" { cfg.EnvPATH = os.Getenv("PATH") } + if cfg.HomeDir == "" { + // os.UserHomeDir uses HOME on Unix and USERPROFILE on Windows. + // A non-empty result gets baked into the service unit so the + // daemon (and any subprocess it spawns) sees the correct home + // even when systemd/launchd would otherwise leave HOME unset. + if home, err := os.UserHomeDir(); err == nil { + cfg.HomeDir = home + } + } if len(cfg.EnvExtra) == 0 { cfg.EnvExtra = captureDaemonEnv(cfg.NoCaptureSecrets) if !cfg.NoCaptureSecrets { @@ -269,4 +279,3 @@ func captureConfigEnvPlaceholdersInString(s string, env map[string]string) { } } } - diff --git a/daemon/systemd.go b/daemon/systemd.go index 4aa046345..d4181300c 100644 --- a/daemon/systemd.go +++ b/daemon/systemd.go @@ -17,6 +17,16 @@ const ( systemdServiceName = ServiceName + ".service" ) +// systemdTemplateOwnedEnvKeys are keys the unit template renders directly; if +// they also appear in cfg.EnvExtra the template version wins. +var systemdTemplateOwnedEnvKeys = map[string]struct{}{ + "CC_LOG_FILE": {}, + "CC_LOG_MAX_SIZE": {}, + "CC_LOG_MAX_BACKUPS": {}, + "PATH": {}, + "HOME": {}, +} + type systemdManager struct { system bool // true = system-level (/etc/systemd/system), false = user-level (~/.config/systemd/user) } @@ -188,6 +198,14 @@ func (m *systemdManager) buildUnit(cfg Config) string { if cfg.EnvPATH != "" { fmt.Fprintf(&sb, "Environment=\"PATH=%s\"\n", cfg.EnvPATH) } + // HOME: systemd does not propagate HOME to system units by default. + // Without this line, os.UserHomeDir() fails in the daemon and any + // subprocess it spawns; config.Load then falls back to a relative + // data_dir which agents (cd'd into work_dir) resolve to the wrong + // place, e.g. /.cc-connect instead of /.cc-connect. + if cfg.HomeDir != "" { + fmt.Fprintf(&sb, "Environment=\"HOME=%s\"\n", escapeSystemdEnvValue(cfg.HomeDir)) + } if len(cfg.EnvExtra) > 0 { keys := make([]string, 0, len(cfg.EnvExtra)) for key := range cfg.EnvExtra { @@ -195,6 +213,9 @@ func (m *systemdManager) buildUnit(cfg Config) string { } sort.Strings(keys) for _, key := range keys { + if _, owned := systemdTemplateOwnedEnvKeys[key]; owned { + continue + } if !isValidEnvName(key) { slog.Warn("daemon: systemd: dropping invalid env name from EnvExtra", "key", key) diff --git a/daemon/systemd_test.go b/daemon/systemd_test.go index 3d3aa57c7..0bf81c74c 100644 --- a/daemon/systemd_test.go +++ b/daemon/systemd_test.go @@ -72,6 +72,66 @@ func TestBuildUnit_DropsEmptyValue(t *testing.T) { } } +// TestBuildUnit_IncludesHOME regresses the "Append system prompt file not +// found" bug: systemd does not inherit HOME from the user's shell for +// system units, so the daemon (and any subprocess it spawns) would fail +// os.UserHomeDir() and config.Load would fall back to a relative +// data_dir that resolves against work_dir instead of the user home. +// The install-time capture of HomeDir is now baked into the unit as an +// Environment= line to guarantee the daemon sees the correct home. +func TestBuildUnit_IncludesHOME(t *testing.T) { + mgr := &systemdManager{system: false} + cfg := Config{ + BinaryPath: "/bin/true", + WorkDir: "/tmp", + LogFile: "/tmp/log", + LogMaxSize: 1024, + EnvPATH: "/usr/bin", + HomeDir: "/home/app", + } + out := mgr.buildUnit(cfg) + if !strings.Contains(out, `Environment="HOME=/home/app"`) { + t.Fatalf("unit should include HOME=/home/app when HomeDir is set; got:\n%s", out) + } +} + +// TestBuildUnit_OmitsHOMEWhenEmpty ensures the unit does not emit an +// empty HOME= line when the caller could not determine one. +func TestBuildUnit_OmitsHOMEWhenEmpty(t *testing.T) { + mgr := &systemdManager{system: false} + cfg := Config{ + BinaryPath: "/bin/true", + WorkDir: "/tmp", + LogFile: "/tmp/log", + LogMaxSize: 1024, + EnvPATH: "/usr/bin", + } + out := mgr.buildUnit(cfg) + if strings.Contains(out, `Environment="HOME=`) { + t.Fatalf("unit should omit HOME= when HomeDir empty; got:\n%s", out) + } +} + +func TestBuildUnit_EnvExtraHOMEDoesNotOverrideTemplateHOME(t *testing.T) { + mgr := &systemdManager{system: false} + cfg := Config{ + BinaryPath: "/bin/true", + WorkDir: "/tmp", + LogFile: "/tmp/log", + LogMaxSize: 1024, + EnvPATH: "/usr/bin", + HomeDir: "/home/app", + EnvExtra: map[string]string{"HOME": "/should-not-override"}, + } + out := mgr.buildUnit(cfg) + if strings.Contains(out, "/should-not-override") { + t.Fatalf("EnvExtra HOME leaked past template ownership: %s", out) + } + if !strings.Contains(out, `Environment="HOME=/home/app"`) { + t.Fatalf("expected template HOME /home/app to survive; got:\n%s", out) + } +} + // TestSystemdInstall_TightensExistingUnitFrom0644 covers the upgrade // path: os.WriteFile would truncate-in-place and KEEP the old 0644 // permissions of a unit file left over from earlier cc-connect diff --git a/tests/e2e/smoke_test.go b/tests/e2e/smoke_test.go index 96d5432ef..9b2e4c599 100644 --- a/tests/e2e/smoke_test.go +++ b/tests/e2e/smoke_test.go @@ -29,6 +29,7 @@ import ( func TestSmoke_ConfigLoading(t *testing.T) { // Create a temporary config file tmpDir := t.TempDir() + t.Setenv("HOME", tmpDir) configPath := filepath.Join(tmpDir, "config.toml") configContent := ` @@ -50,12 +51,14 @@ level = "info" require.NoError(t, err) // Load the config + oldConfigPath := config.ConfigPath + t.Cleanup(func() { config.ConfigPath = oldConfigPath }) config.ConfigPath = configPath cfg, err := config.Load(configPath) require.NoError(t, err) // Verify basic config fields - assert.Equal(t, "~/.cc-connect", cfg.DataDir) + assert.Equal(t, filepath.Join(tmpDir, ".cc-connect"), cfg.DataDir) assert.Len(t, cfg.Projects, 1) assert.Equal(t, "test-project", cfg.Projects[0].Name) assert.Equal(t, "claudecode", cfg.Projects[0].Agent.Type) @@ -374,10 +377,10 @@ func TestSmoke_EventTypes(t *testing.T) { {Type: core.EventResult, Content: "final result", Done: true}, {Type: core.EventError, Error: context.DeadlineExceeded, Done: true}, { - Type: core.EventPermissionRequest, - ToolName: "Bash", - ToolInput: "rm -rf /", - RequestID: "req-001", + Type: core.EventPermissionRequest, + ToolName: "Bash", + ToolInput: "rm -rf /", + RequestID: "req-001", }, } @@ -395,14 +398,14 @@ func TestSmoke_EventTypes(t *testing.T) { func TestSmoke_WorkspaceSwitch(t *testing.T) { // Create simple workspace state maps to verify isolation concept ws1 := map[string]string{ - "id": "workspace-1", - "session": "session-A", - "agent": "claudecode", + "id": "workspace-1", + "session": "session-A", + "agent": "claudecode", } ws2 := map[string]string{ - "id": "workspace-2", - "session": "session-B", - "agent": "gemini", + "id": "workspace-2", + "session": "session-B", + "agent": "gemini", } assert.Equal(t, "workspace-1", ws1["id"]) @@ -559,4 +562,3 @@ func TestSmoke_WebhookCallback(t *testing.T) { t.Log("Webhook callback: PASS") } -