diff --git a/internal/module/workspace.go b/internal/module/workspace.go index 0a9a577f..e69fc88e 100644 --- a/internal/module/workspace.go +++ b/internal/module/workspace.go @@ -2,7 +2,10 @@ package module import ( "context" + "errors" "fmt" + "io/fs" + "os" "path/filepath" "strings" @@ -13,8 +16,10 @@ import ( // WorkspaceModule manages workspace symlink federation and shell config. // // Symlink safety rules: -// - Only create symlinks when the link path does NOT exist at all. -// - Never delete, overwrite, or modify existing symlinks or directories. +// - Create a symlink only when the link path does not exist at all, or holds a +// dangling symlink (its target is gone) that is removed first. +// - Never delete, overwrite, or modify real files, directories, or symlinks +// whose target still exists. // - Broken symlinks are repaired only if an explicit target is configured. // // Dual-workspace architecture: @@ -159,6 +164,9 @@ func (m *WorkspaceModule) Apply(ctx context.Context, rc *RunContext) (*ApplyResu if !m.targetReachable(rc, symlink) { return nil, fmt.Errorf("symlink target does not exist: %s", cfg.Symlink) } + if err := m.removeDanglingSymlink(rc, workspacePath); err != nil { + return nil, fmt.Errorf("replacing dangling workspace symlink: %w", err) + } if err := rc.Runner.Symlink(symlink, workspacePath); err != nil { return nil, fmt.Errorf("symlinking workspace: %w", err) } @@ -172,6 +180,9 @@ func (m *WorkspaceModule) Apply(ctx context.Context, rc *RunContext) (*ApplyResu if !m.targetReachable(rc, gdrive) { return nil, fmt.Errorf("gdrive target does not exist: %s", cfg.Gdrive) } + if err := m.removeDanglingSymlink(rc, gdriveSymlink); err != nil { + return nil, fmt.Errorf("replacing dangling gdrive symlink: %w", err) + } if err := rc.Runner.Symlink(gdrive, gdriveSymlink); err != nil { return nil, fmt.Errorf("symlinking gdrive: %w", err) } @@ -186,6 +197,9 @@ func (m *WorkspaceModule) Apply(ctx context.Context, rc *RunContext) (*ApplyResu gdriveWork := filepath.Join(gdriveSymlink, "work") gdriveXref := filepath.Join(workDir, ".gdrive") if m.targetReachable(rc, gdriveWork) && !m.pathUsable(rc, gdriveXref) { + if err := m.removeDanglingSymlink(rc, gdriveXref); err != nil { + return nil, fmt.Errorf("replacing dangling work/.gdrive symlink: %w", err) + } if err := rc.Runner.Symlink(gdriveWork, gdriveXref); err != nil { return nil, fmt.Errorf("symlinking work/.gdrive: %w", err) } @@ -201,6 +215,9 @@ func (m *WorkspaceModule) Apply(ctx context.Context, rc *RunContext) (*ApplyResu linkPath := filepath.Join(inboxDir, sub) gdriveTarget := filepath.Join(gdriveSymlink, "work", "inbox", sub) if m.targetReachable(rc, gdriveTarget) && !m.pathUsable(rc, linkPath) { + if err := m.removeDanglingSymlink(rc, linkPath); err != nil { + return nil, fmt.Errorf("replacing dangling inbox/%s symlink: %w", sub, err) + } if err := rc.Runner.Symlink(gdriveTarget, linkPath); err != nil { return nil, fmt.Errorf("symlinking inbox/%s: %w", sub, err) } @@ -249,3 +266,28 @@ func (m *WorkspaceModule) pathUsable(rc *RunContext, path string) bool { func (m *WorkspaceModule) targetReachable(rc *RunContext, path string) bool { return rc.Runner.FileExists(path) || rc.Runner.IsDir(path) } + +// removeDanglingSymlink removes path only when it is a symlink whose target is +// missing, so the Symlink call that follows can create the link Apply planned. +// A real file or directory, and a symlink whose target exists, are left alone. +// Removal goes through Runner.Remove, so a dry run removes nothing. +// +// Only a target confirmed missing (fs.ErrNotExist) counts as dangling. Any other +// stat failure, such as a target behind an untraversable directory or an +// unavailable mount, leaves the link in place and is returned as an error. +func (m *WorkspaceModule) removeDanglingSymlink(rc *RunContext, path string) error { + if !rc.Runner.IsSymlink(path) { + return nil + } + _, statErr := os.Stat(path) + if statErr == nil { + return nil + } + if !errors.Is(statErr, fs.ErrNotExist) { + return fmt.Errorf("cannot tell whether the target of symlink %s exists (%v); left it in place, check the link target's permissions or mount", path, statErr) + } + if err := rc.Runner.Remove(path); err != nil && !os.IsNotExist(err) { + return err + } + return nil +} diff --git a/internal/module/workspace_test.go b/internal/module/workspace_test.go new file mode 100644 index 00000000..dbd0ec41 --- /dev/null +++ b/internal/module/workspace_test.go @@ -0,0 +1,160 @@ +package module + +import ( + "context" + "io" + "log/slog" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/STAIxBWLB/dotfiles/internal/config" + dotexec "github.com/STAIxBWLB/dotfiles/internal/exec" + "github.com/STAIxBWLB/dotfiles/internal/template" +) + +// workspaceApplyFixture builds a workspace whose cloud root already holds +// work/inbox/{downloads,incoming}, so Apply only has to federate symlinks. +func workspaceApplyFixture(t *testing.T) (*RunContext, string) { + t.Helper() + home := t.TempDir() + drive := filepath.Join(home, "drive") + for _, dir := range []string{ + filepath.Join(drive, "work", "inbox", "downloads"), + filepath.Join(drive, "work", "inbox", "incoming"), + filepath.Join(home, "workspace", "work"), + } { + if err := os.MkdirAll(dir, 0o755); err != nil { + t.Fatal(err) + } + } + + rc := &RunContext{ + Config: &config.Config{Modules: config.ModulesConfig{Workspace: config.WorkConfig{ + Enabled: true, + Path: filepath.Join(home, "workspace"), + Gdrive: drive, + GdriveSymlink: filepath.Join(home, "gdrive-workspace"), + }}}, + Runner: dotexec.NewRunner(false, slog.New(slog.NewTextHandler(io.Discard, nil))), + Template: template.NewEngine(), + HomeDir: home, + Out: io.Discard, + } + return rc, home +} + +// A link whose target is gone is what a moved cloud mirror leaves behind: +// Check plans the relink and Apply must remove the stale link before creating it. +func TestWorkspaceApplyReplacesDanglingGdriveLink(t *testing.T) { + rc, home := workspaceApplyFixture(t) + gdriveXref := filepath.Join(home, "workspace", "work", ".gdrive") + if err := os.Symlink(filepath.Join(home, "old-drive", "work"), gdriveXref); err != nil { + t.Fatal(err) + } + + if _, err := (&WorkspaceModule{}).Apply(context.Background(), rc); err != nil { + t.Fatalf("Apply: %v", err) + } + + want := filepath.Join(home, "gdrive-workspace", "work") + got, err := os.Readlink(gdriveXref) + if err != nil { + t.Fatalf("work/.gdrive is not a symlink after Apply: %v", err) + } + if got != want { + t.Errorf("work/.gdrive -> %q, want %q", got, want) + } + if info, err := os.Stat(gdriveXref); err != nil || !info.IsDir() { + t.Errorf("work/.gdrive does not resolve to a directory: info=%v err=%v", info, err) + } + + check, err := (&WorkspaceModule{}).Check(context.Background(), rc) + if err != nil { + t.Fatalf("Check after Apply: %v", err) + } + if !check.Satisfied { + t.Errorf("Check after Apply is not satisfied: %+v", check.Changes) + } +} + +// A link whose target cannot be stat'ed for a reason other than "missing" +// (here an untraversable parent) is not dangling: it stays, and Apply errors. +func TestWorkspaceApplyKeepsSymlinkWithUnreadableTarget(t *testing.T) { + if os.Geteuid() == 0 { + t.Skip("root traverses a 0o000 directory") + } + rc, home := workspaceApplyFixture(t) + locked := filepath.Join(home, "locked") + target := filepath.Join(locked, "work") + if err := os.MkdirAll(target, 0o755); err != nil { + t.Fatal(err) + } + if err := os.Chmod(locked, 0o000); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = os.Chmod(locked, 0o755) }) + gdriveXref := filepath.Join(home, "workspace", "work", ".gdrive") + if err := os.Symlink(target, gdriveXref); err != nil { + t.Fatal(err) + } + + _, err := (&WorkspaceModule{}).Apply(context.Background(), rc) + if err == nil || !strings.Contains(err.Error(), gdriveXref) || !strings.Contains(err.Error(), "permissions or mount") { + t.Fatalf("Apply error = %v, want one naming %s and the permissions/mount hint", err, gdriveXref) + } + got, lerr := os.Readlink(gdriveXref) + if lerr != nil || got != target { + t.Fatalf("link was changed: target=%q err=%v, want %q", got, lerr, target) + } +} + +// A link whose target exists is the healthy state and must not be touched. +func TestWorkspaceApplyLeavesLiveSymlinkAlone(t *testing.T) { + rc, home := workspaceApplyFixture(t) + target := filepath.Join(home, "live-target") + if err := os.MkdirAll(target, 0o755); err != nil { + t.Fatal(err) + } + gdriveXref := filepath.Join(home, "workspace", "work", ".gdrive") + if err := os.Symlink(target, gdriveXref); err != nil { + t.Fatal(err) + } + + if _, err := (&WorkspaceModule{}).Apply(context.Background(), rc); err != nil { + t.Fatalf("Apply: %v", err) + } + + got, err := os.Readlink(gdriveXref) + if err != nil { + t.Fatalf("work/.gdrive is not a symlink after Apply: %v", err) + } + if got != target { + t.Errorf("work/.gdrive -> %q, want unchanged %q", got, target) + } +} + +// A real directory at the link path is user data: Apply must never replace it. +func TestWorkspaceApplyLeavesRealDirectoryAlone(t *testing.T) { + rc, home := workspaceApplyFixture(t) + gdriveXref := filepath.Join(home, "workspace", "work", ".gdrive") + if err := os.MkdirAll(gdriveXref, 0o755); err != nil { + t.Fatal(err) + } + marker := filepath.Join(gdriveXref, "keep.txt") + if err := os.WriteFile(marker, []byte("user data\n"), 0o644); err != nil { + t.Fatal(err) + } + + if _, err := (&WorkspaceModule{}).Apply(context.Background(), rc); err != nil { + t.Fatalf("Apply: %v", err) + } + + if info, err := os.Lstat(gdriveXref); err != nil || !info.IsDir() { + t.Fatalf("work/.gdrive is no longer a real directory: info=%v err=%v", info, err) + } + if _, err := os.Stat(marker); err != nil { + t.Errorf("file inside the real directory was lost: %v", err) + } +}