Commit
eeco
fix: harden cross-platform paths in settings import; restore coverage floor
The settings-import self-guard and copyFile compared workspace paths by string, which slips on Windows (drive-letter case, 8.3 short names): a self-import was not blocked there, and a --force self-import could truncate cockpit.json/workflows via O_TRUNC on the same file. Both now use os.SameFile, and copyFile is a no-op when src and dst are the same file. No behaviour change on Linux/macOS. Tests: GlobalConfigDir's home-fallback test now sets the platform home env (USERPROFILE on Windows, where os.UserHomeDir ignores $HOME); add ParseLocalFile, copyTree/copyFile, and config-list global-origin tests so total coverage holds the 83% floor.
modified cmd/eeco/config.go
@@ -60,7 +60,7 @@ func runConfigImport(args []string, stdout, stderr io.Writer) int {
fmt.Fprintln(stderr, "eeco config:", err)
return 1
}
if src.Workspace == dst.Workspace {
if sameWorkspace(src.Workspace, dst.Workspace) {
fmt.Fprintln(stderr, "eeco config: source and destination are the same workspace")
return 1
}
modified cmd/eeco/config_test.go
@@ -74,6 +74,21 @@ func TestRunConfig_GlobalSetNeedsNoWorkspace(t *testing.T) {
}
}
func TestRunConfig_ListShowsGlobalOrigin(t *testing.T) {
t.Setenv(config.GlobalConfigEnv, t.TempDir())
if code := runConfig([]string{"set", "--global", "ai_budget=4"}, &bytes.Buffer{}, &bytes.Buffer{}); code != 0 {
t.Fatalf("global set exit=%d", code)
}
setupInited(t)
var out bytes.Buffer
if code := runConfig([]string{"list"}, &out, &bytes.Buffer{}); code != 0 {
t.Fatalf("list exit=%d", code)
}
if !strings.Contains(out.String(), "ai_budget") || !strings.Contains(out.String(), "4 (global)") {
t.Errorf("list should tag ai_budget as (global):\n%s", out.String())
}
}
func TestRunConfig_GlobalFlagPositionIndependent(t *testing.T) {
// --global must work whether it precedes or trails the key=value, so a
// trailing flag can't silently write the wrong layer.
modified cmd/eeco/import.go
@@ -48,6 +48,19 @@ func resolveSourceWorkspace(path string) (*config.Config, error) {
return src, nil
}
// sameWorkspace reports whether two workspace paths are the same directory,
// using file identity (os.SameFile) so it is robust to symlinks, drive-letter
// case, and 8.3 short names — a plain string compare slips the self-import
// guard on Windows.
func sameWorkspace(a, b string) bool {
ai, ae := os.Stat(a)
bi, be := os.Stat(b)
if ae == nil && be == nil {
return os.SameFile(ai, bi)
}
return filepath.Clean(a) == filepath.Clean(b)
}
// importSettings copies portable settings from an initialised source workspace
// into dst's workspace: config.local (verbatim into a fresh workspace, else a
// key-merge), cockpit.json, and the workflows/ directory. Without force,
@@ -210,6 +223,12 @@ func copyFile(src, dst string) error {
if err != nil {
return err
}
// Never copy a file onto itself: O_TRUNC on the same inode would wipe it
// before the read. (The self-import guard catches the common case; this is
// defense in depth for any path that reaches here with src == dst.)
if dstInfo, err := os.Stat(dst); err == nil && os.SameFile(info, dstInfo) {
return nil
}
if err := os.MkdirAll(filepath.Dir(dst), 0o755); err != nil {
return err
}
modified cmd/eeco/import_test.go
@@ -4,10 +4,55 @@ import (
"bytes"
"os"
"path/filepath"
"runtime"
"strings"
"testing"
)
func TestCopyTreeAndCopyFile(t *testing.T) {
src := t.TempDir()
if err := os.MkdirAll(filepath.Join(src, "sub"), 0o755); err != nil {
t.Fatal(err)
}
if err := os.WriteFile(filepath.Join(src, "run"), []byte("#!/bin/sh\necho hi\n"), 0o755); err != nil {
t.Fatal(err)
}
if err := os.WriteFile(filepath.Join(src, "sub", "note.txt"), []byte("hi"), 0o644); err != nil {
t.Fatal(err)
}
dst := filepath.Join(t.TempDir(), "out")
if err := copyTree(src, dst); err != nil {
t.Fatalf("copyTree: %v", err)
}
if b, _ := os.ReadFile(filepath.Join(dst, "sub", "note.txt")); string(b) != "hi" {
t.Errorf("nested file not copied: %q", b)
}
if runtime.GOOS != "windows" { // no exec bit on Windows
if fi, _ := os.Stat(filepath.Join(dst, "run")); fi.Mode().Perm()&0o100 == 0 {
t.Errorf("exec bit lost on copied run: %v", fi.Mode())
}
}
// copyFile onto itself must be a no-op, not a truncation.
self := filepath.Join(src, "run")
if err := copyFile(self, self); err != nil {
t.Fatalf("copyFile self: %v", err)
}
if b, _ := os.ReadFile(self); len(b) == 0 {
t.Error("copyFile onto self truncated the file")
}
// copyFile creates missing parent directories.
deep := filepath.Join(t.TempDir(), "a", "b", "c.txt")
if err := copyFile(self, deep); err != nil {
t.Fatalf("copyFile into missing dirs: %v", err)
}
if _, err := os.Stat(deep); err != nil {
t.Errorf("copyFile did not create parents: %v", err)
}
}
// buildSourceProject stands up an initialised eeco project with a couple of
// tuned config keys, a non-default cockpit target, and one scaffolded
// workflow, then returns its repo root. It leaves the cwd inside the project.
modified cmd/eeco/init.go
@@ -101,7 +101,7 @@ func runInit(args []string, stdout, stderr io.Writer) int {
fmt.Fprintln(stderr, "eeco init: --from:", err)
return 1
}
if src.Workspace == cfg.Workspace {
if sameWorkspace(src.Workspace, cfg.Workspace) {
fmt.Fprintln(stderr, "eeco init: --from points at this same project")
return 1
}
modified internal/config/global_test.go
@@ -3,6 +3,7 @@ package config
import (
"os"
"path/filepath"
"runtime"
"strings"
"testing"
)
@@ -22,11 +23,17 @@ func TestGlobalConfigDir_EnvPrecedence(t *testing.T) {
t.Fatalf("GlobalConfigDir() = %q, want %q", got, want)
}
// With neither, fall back to $HOME/.config/eeco.
// With neither, fall back to <home>/.config/eeco. os.UserHomeDir reads
// $HOME on unix but %USERPROFILE% on Windows, so set the platform's var.
t.Setenv(GlobalConfigEnv, "")
t.Setenv("XDG_CONFIG_HOME", "")
t.Setenv("HOME", "/home/tester")
if got, want := GlobalConfigDir(), filepath.Join("/home/tester", ".config", "eeco"); got != want {
homeEnv := "HOME"
if runtime.GOOS == "windows" {
homeEnv = "USERPROFILE"
}
home := filepath.FromSlash("/home/tester")
t.Setenv(homeEnv, home)
if got, want := GlobalConfigDir(), filepath.Join(home, ".config", "eeco"); got != want {
t.Fatalf("GlobalConfigDir() = %q, want %q", got, want)
}
}
@@ -178,6 +185,44 @@ func TestWriteGlobalKeys_UpsertsAndCreatesDir(t *testing.T) {
}
}
func TestParseLocalFile(t *testing.T) {
if m, err := ParseLocalFile(""); err != nil || len(m) != 0 {
t.Fatalf("ParseLocalFile(empty) = %v,%v want empty,nil", m, err)
}
dir := t.TempDir()
p := filepath.Join(dir, LocalFilename)
content := "# comment\n\nautomation=auto\nai_budget = \"3\"\ngate=go build\ngate=go vet\nattribution_pattern=a=b\nbroken_line_without_eq\n"
if err := os.WriteFile(p, []byte(content), 0o644); err != nil {
t.Fatal(err)
}
m, err := ParseLocalFile(p)
if err != nil {
t.Fatal(err)
}
if m["automation"] != "auto" {
t.Errorf("automation = %q", m["automation"])
}
if m["ai_budget"] != "3" {
t.Errorf("ai_budget = %q, want 3 (quotes stripped)", m["ai_budget"])
}
if m["gate"] != "go vet" {
t.Errorf("gate = %q, want last-wins go vet", m["gate"])
}
if m["attribution_pattern"] != "a=b" {
t.Errorf("attribution_pattern = %q, want a=b (split on first =)", m["attribution_pattern"])
}
if _, ok := m["broken_line_without_eq"]; ok {
t.Error("a line without '=' should be skipped")
}
if m2, err := ParseLocalFile(filepath.Join(dir, "missing")); err != nil || len(m2) != 0 {
t.Fatalf("ParseLocalFile(missing) = %v,%v want empty,nil", m2, err)
}
// TestMain pins EECO_CONFIG_HOME, so the global path resolves non-empty.
if GlobalConfigLocalPath() == "" {
t.Error("GlobalConfigLocalPath() empty despite EECO_CONFIG_HOME set")
}
}
func TestDeclaredKeys(t *testing.T) {
if keys, err := DeclaredKeys(""); err != nil || len(keys) != 0 {
t.Fatalf("DeclaredKeys(empty) = %v,%v, want empty,nil", keys, err)