fix(worktree): reject stale task path reuse (#3013)

Co-authored-by: ayattara <alassane.yattara@savoirfairelinux.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Carlos Florencio <carlosmflorencio@gmail.com>
This commit is contained in:
Alassane
2026-08-26 22:08:01 +00:00
committed by GitHub
parent 3febedd6ba
commit 898225214b
16 changed files with 1652 additions and 10 deletions
@@ -456,6 +456,146 @@ func TestWorktreePreparer_MultiRepo_RollbackRemovesWorktreeCreatedForStaleReuseI
}
}
// TestWorktreePreparer_FreshStartRejectsStaleWorktreePathOwnedByLiveTask
// protects the fresh-start path from a stale worktree record whose on-disk
// path has since been claimed by another live task. The stale task must fail
// safely; it must never attach to or change the live task's checkout.
func TestWorktreePreparer_FreshStartRejectsStaleWorktreePathOwnedByLiveTask(t *testing.T) {
repositoryPath := initBareGitRepo(t, "shared-repository")
preparer, manager, store := newPreparerForTestWithStore(t)
ctx := context.Background()
live, err := manager.Create(ctx, worktree.CreateRequest{
TaskID: "task-live",
SessionID: "session-live",
TaskTitle: "Live task",
RepositoryID: "repository-shared",
RepositoryPath: repositoryPath,
BaseBranch: "main",
TaskDirName: "live-task_abc",
RepoName: "shared-repository",
})
if err != nil {
t.Fatalf("create live worktree: %v", err)
}
liveHead := strings.TrimSpace(runGitForPreparerTest(t, live.Path, "rev-parse", "HEAD"))
if err := store.CreateWorktree(ctx, &worktree.Worktree{
ID: "worktree-stale",
TaskID: "task-stale",
SessionID: "session-stale",
RepositoryID: "repository-shared",
Path: live.Path,
Branch: "feat/stale",
Status: worktree.StatusActive,
}); err != nil {
t.Fatalf("seed stale worktree: %v", err)
}
result, err := preparer.Prepare(ctx, &EnvPrepareRequest{
TaskID: "task-stale",
SessionID: "session-stale",
TaskTitle: "Stale task",
ExecutorType: executor.NameStandalone,
RepositoryID: "repository-shared",
RepositoryPath: repositoryPath,
BaseBranch: "main",
WorktreeID: "worktree-stale",
TaskDirName: "stale-task_def",
RepoName: "shared-repository",
}, nil)
if err != nil {
t.Fatalf("prepare returned hard error: %v", err)
}
if result.Success {
t.Fatal("fresh start succeeded by reusing a live task's worktree path")
}
if !errors.Is(result.Error, worktree.ErrWorktreePathOwnedByAnotherTask) {
t.Fatalf("prepare error = %v, want ErrWorktreePathOwnedByAnotherTask", result.Error)
}
if got := strings.TrimSpace(runGitForPreparerTest(t, live.Path, "rev-parse", "HEAD")); got != liveHead {
t.Fatalf("live worktree HEAD changed: got %q, want %q", got, liveHead)
}
if !manager.IsValid(live.Path) {
t.Fatal("live worktree was removed or corrupted")
}
}
// TestWorktreePreparer_FreshStartRejectsStaleWorktreePathOwnedByLiveTask_WorktreeIDOnly
// exercises the WorktreeID-only lookup branch of tryReuseExisting (when
// SessionID is empty), confirming the ownership check fires there too.
func TestWorktreePreparer_FreshStartRejectsStaleWorktreePathOwnedByLiveTask_WorktreeIDOnly(t *testing.T) {
repositoryPath := initBareGitRepo(t, "shared-repository")
preparer, manager, store := newPreparerForTestWithStore(t)
ctx := context.Background()
live, err := manager.Create(ctx, worktree.CreateRequest{
TaskID: "task-live",
SessionID: "session-live",
TaskTitle: "Live task",
RepositoryID: "repository-shared",
RepositoryPath: repositoryPath,
BaseBranch: "main",
TaskDirName: "live-task_wtid",
RepoName: "shared-repository",
})
if err != nil {
t.Fatalf("create live worktree: %v", err)
}
liveHead := strings.TrimSpace(runGitForPreparerTest(t, live.Path, "rev-parse", "HEAD"))
if err := store.CreateWorktree(ctx, &worktree.Worktree{
ID: "worktree-stale-wtid",
TaskID: "task-stale-wtid",
SessionID: "session-stale-wtid",
RepositoryID: "repository-shared",
Path: live.Path,
Branch: "feat/stale-wtid",
Status: worktree.StatusActive,
}); err != nil {
t.Fatalf("seed stale worktree: %v", err)
}
result, err := preparer.Prepare(ctx, &EnvPrepareRequest{
TaskID: "task-stale-wtid",
SessionID: "", // empty — forces WorktreeID-only lookup
TaskTitle: "Stale task WTID-only",
ExecutorType: executor.NameStandalone,
RepositoryID: "repository-shared",
RepositoryPath: repositoryPath,
BaseBranch: "main",
WorktreeID: "worktree-stale-wtid",
TaskDirName: "stale-task-wtid_xyz",
RepoName: "shared-repository",
}, nil)
if err != nil {
t.Fatalf("prepare returned hard error: %v", err)
}
if result.Success {
t.Fatal("fresh start succeeded by reusing a live task's worktree path via WorktreeID-only lookup")
}
if !errors.Is(result.Error, worktree.ErrWorktreePathOwnedByAnotherTask) {
t.Fatalf("prepare error = %v, want ErrWorktreePathOwnedByAnotherTask", result.Error)
}
if got := strings.TrimSpace(runGitForPreparerTest(t, live.Path, "rev-parse", "HEAD")); got != liveHead {
t.Fatalf("live worktree HEAD changed: got %q, want %q", got, liveHead)
}
if !manager.IsValid(live.Path) {
t.Fatal("live worktree was removed or corrupted")
}
}
func runGitForPreparerTest(t *testing.T, directory string, args ...string) string {
t.Helper()
command := exec.Command("git", args...)
command.Dir = directory
output, err := command.CombinedOutput()
if err != nil {
t.Fatalf("git %s in %s failed: %v\n%s", strings.Join(args, " "), directory, err, output)
}
return string(output)
}
func TestWorktreePreparer_MultiRepo_RunsPerRepoSetupScript(t *testing.T) {
repoA := initBareGitRepo(t, "frontend")
repoB := initBareGitRepo(t, "backend")
@@ -509,3 +649,76 @@ func TestWorktreePreparer_MultiRepo_RunsPerRepoSetupScript(t *testing.T) {
}
}
}
// TestWorktreePreparer_FreshStartRejectsStaleWorktreePath_NestedProjectMarker
// protects against the ownership-marker walk being confused by a legitimate
// .kandev-workspace.json file inside a descendant repository checkout. The
// stale task must still be rejected — the presence of a non-task-root marker
// must not shadow the task-root marker.
func TestWorktreePreparer_FreshStartRejectsStaleWorktreePath_NestedProjectMarker(t *testing.T) {
repositoryPath := initBareGitRepo(t, "repo-with-marker")
preparer, manager, store := newPreparerForTestWithStore(t)
ctx := context.Background()
live, err := manager.Create(ctx, worktree.CreateRequest{
TaskID: "task-live-nested",
SessionID: "session-live-nested",
TaskTitle: "Live task",
RepositoryID: "repository-nested",
RepositoryPath: repositoryPath,
BaseBranch: "main",
TaskDirName: "live-task_nested",
RepoName: "repo-with-marker",
})
if err != nil {
t.Fatalf("create live worktree: %v", err)
}
liveHead := strings.TrimSpace(runGitForPreparerTest(t, live.Path, "rev-parse", "HEAD"))
// Write a legitimate .kandev-workspace.json inside the repo checkout itself
// (as a real project might) to see if it shadows the task-root marker.
if err := os.WriteFile(filepath.Join(live.Path, ".kandev-workspace.json"),
[]byte(`{"project":"not-kandev","version":1}`), 0644); err != nil {
t.Fatalf("write project marker: %v", err)
}
if err := store.CreateWorktree(ctx, &worktree.Worktree{
ID: "worktree-stale-nested",
TaskID: "task-stale-nested",
SessionID: "session-stale-nested",
RepositoryID: "repository-nested",
Path: live.Path,
Branch: "feat/stale-nested",
Status: worktree.StatusActive,
}); err != nil {
t.Fatalf("seed stale worktree: %v", err)
}
result, err := preparer.Prepare(ctx, &EnvPrepareRequest{
TaskID: "task-stale-nested",
SessionID: "", // empty — forces WorktreeID-only lookup
TaskTitle: "Stale task nested-marker",
ExecutorType: executor.NameStandalone,
RepositoryID: "repository-nested",
RepositoryPath: repositoryPath,
BaseBranch: "main",
WorktreeID: "worktree-stale-nested",
TaskDirName: "stale-task-nested_def",
RepoName: "repo-with-marker",
}, nil)
if err != nil {
t.Fatalf("prepare returned hard error: %v", err)
}
if result.Success {
t.Fatal("fresh start succeeded despite nested non-ownership marker in repo checkout")
}
if !errors.Is(result.Error, worktree.ErrWorktreePathOwnedByAnotherTask) {
t.Fatalf("prepare error = %v, want ErrWorktreePathOwnedByAnotherTask", result.Error)
}
if got := strings.TrimSpace(runGitForPreparerTest(t, live.Path, "rev-parse", "HEAD")); got != liveHead {
t.Fatalf("live worktree HEAD changed: got %q, want %q", got, liveHead)
}
if !manager.IsValid(live.Path) {
t.Fatal("live worktree was removed or corrupted")
}
}
@@ -16,6 +16,13 @@ import (
const dependencyDirectoryOpenFlags = unix.O_RDONLY | unix.O_DIRECTORY | unix.O_NOFOLLOW | unix.O_CLOEXEC
// RemoveDirectoryNoFollow opens every path component without following links and
// removes the target through directory descriptors. It is safe against a
// concurrent rename or symlink replacement redirecting deletion outside root.
func RemoveDirectoryNoFollow(ctx context.Context, root, target string) error {
return removeDependencyDirectory(ctx, root, target)
}
// removeDependencyDirectory opens every path component with O_NOFOLLOW and removes
// entries through the resulting directory descriptors. This keeps a concurrent
// rename or symlink replacement from redirecting deletion outside the workspace.
@@ -15,6 +15,12 @@ import (
"golang.org/x/sys/windows"
)
// RemoveDirectoryNoFollow opens every path component without following links and
// removes the target through the resulting native handles.
func RemoveDirectoryNoFollow(ctx context.Context, root, target string) error {
return removeDependencyDirectory(ctx, root, target)
}
// Windows uses native handles rooted at the workspace. OBJ_DONT_REPARSE and
// FILE_OPEN_REPARSE_POINT make each component no-follow; deletion is then
// requested on the opened handle rather than by path.
@@ -102,6 +108,15 @@ func openWindowsDependencyHandle(
parent windows.Handle,
name string,
createOption uint32,
) (windows.Handle, error) {
return openWindowsDependencyHandleWithDisposition(parent, name, windows.FILE_OPEN, createOption)
}
func openWindowsDependencyHandleWithDisposition(
parent windows.Handle,
name string,
disposition uint32,
createOption uint32,
) (windows.Handle, error) {
objectName, err := windows.NewNTUnicodeString(name)
if err != nil {
@@ -124,7 +139,7 @@ func openWindowsDependencyHandle(
&allocationSize,
0,
windows.FILE_SHARE_READ|windows.FILE_SHARE_WRITE|windows.FILE_SHARE_DELETE,
windows.FILE_OPEN,
disposition,
createOption|windows.FILE_OPEN_REPARSE_POINT,
0,
0,
@@ -0,0 +1,81 @@
package workspaces
import (
"encoding/json"
"errors"
"fmt"
"os"
"strings"
)
// DirectoryHandle keeps a directory pinned while a worktree decision is made.
// Implementations open every path component without following links and use
// the resulting descriptor or native handle for subsequent reads and writes.
// This prevents a path replacement between validation and the side effect
// from redirecting the operation to another task root.
type DirectoryHandle interface {
Close() error
VerifyPath(path string) error
IsValidWorktree() bool
ReadFile(name string) ([]byte, error)
WriteFile(name string, data []byte, mode os.FileMode) error
}
// WriteOwnershipMarkerNoFollow writes an ownership marker through an already
// opened task-root handle. The caller must verify the handle still identifies
// the expected lexical path before calling this function.
func WriteOwnershipMarkerNoFollow(handle DirectoryHandle, marker OwnershipMarker) error {
if handle == nil {
return errors.New("workspace ownership marker requires an opened directory")
}
if err := normalizeOwnershipMarker(&marker); err != nil {
return err
}
matched, err := existingMarkerMatchesHandle(handle, marker)
if err != nil {
return err
}
if matched {
return nil
}
encoded, err := json.Marshal(marker)
if err != nil {
return fmt.Errorf("encode workspace ownership marker: %w", err)
}
if err := handle.WriteFile(OwnershipMarkerFilename, encoded, 0o600); err != nil {
return fmt.Errorf("install workspace ownership marker: %w", err)
}
return nil
}
func existingMarkerMatchesHandle(handle DirectoryHandle, marker OwnershipMarker) (bool, error) {
encoded, err := handle.ReadFile(OwnershipMarkerFilename)
if errors.Is(err, os.ErrNotExist) {
return false, nil
}
if err != nil {
return false, err
}
var existing OwnershipMarker
if err := json.Unmarshal(encoded, &existing); err != nil {
return false, fmt.Errorf("decode workspace ownership marker: %w", err)
}
if existing.TaskID == "" || existing.TaskDirName == "" ||
(existing.LayoutVersion != LayoutVersionSemantic && existing.LayoutVersion != LayoutVersionScratch) {
return false, errors.New("invalid workspace ownership marker fields")
}
if existing.TaskID != marker.TaskID || existing.TaskDirName != marker.TaskDirName || existing.LayoutVersion != marker.LayoutVersion {
return false, errors.New("workspace ownership marker conflicts with requested task root")
}
if existing.WorkspaceID != "" && marker.WorkspaceID != "" && existing.WorkspaceID != marker.WorkspaceID {
return false, errors.New("workspace ownership marker conflicts with requested workspace")
}
return true, nil
}
func validateDirectoryEntryName(name string) error {
if name == "" || name == "." || name == ".." || strings.ContainsAny(name, "/\\\x00") {
return fmt.Errorf("invalid directory entry name: %q", name)
}
return nil
}
@@ -0,0 +1,29 @@
//go:build !windows
package workspaces
import (
"os"
"path/filepath"
"testing"
)
func TestCreateDirectoryNoFollowRejectsSymlinkedManagedRoot(t *testing.T) {
parent := t.TempDir()
tasksBase := filepath.Join(parent, "tasks")
outside := filepath.Join(parent, "outside")
if err := os.MkdirAll(outside, 0o755); err != nil {
t.Fatalf("mkdir outside: %v", err)
}
if err := os.Symlink(outside, tasksBase); err != nil {
t.Fatalf("symlink tasks base: %v", err)
}
_, err := CreateDirectoryNoFollow(tasksBase, filepath.Join(tasksBase, "task-one"), 0o755)
if err == nil {
t.Fatal("CreateDirectoryNoFollow followed a symlinked tasks base")
}
if _, statErr := os.Lstat(filepath.Join(outside, "task-one")); !os.IsNotExist(statErr) {
t.Fatalf("symlink target was modified: %v", statErr)
}
}
@@ -0,0 +1,84 @@
package workspaces
import (
"os"
"path/filepath"
"testing"
)
func TestDirectoryHandlePinsWorktreeAcrossPathReplacement(t *testing.T) {
parent := t.TempDir()
original := filepath.Join(parent, "original")
replacement := filepath.Join(parent, "replacement")
if err := os.MkdirAll(original, 0o755); err != nil {
t.Fatalf("mkdir original: %v", err)
}
if err := os.MkdirAll(replacement, 0o755); err != nil {
t.Fatalf("mkdir replacement: %v", err)
}
if err := os.WriteFile(filepath.Join(original, ".git"), []byte("gitdir: original\n"), 0o600); err != nil {
t.Fatalf("write original git file: %v", err)
}
if err := os.WriteFile(filepath.Join(replacement, ".git"), []byte("gitdir: replacement\n"), 0o600); err != nil {
t.Fatalf("write replacement git file: %v", err)
}
handle, err := OpenDirectoryNoFollow(parent, original)
if err != nil {
t.Fatalf("open original directory: %v", err)
}
t.Cleanup(func() { _ = handle.Close() })
archived := original + ".archived"
if err := os.Rename(original, archived); err != nil {
t.Fatalf("rename original directory: %v", err)
}
if err := os.Rename(replacement, original); err != nil {
t.Fatalf("replace original directory: %v", err)
}
if !handle.IsValidWorktree() {
t.Fatal("opened directory no longer validates as the original worktree")
}
if err := handle.VerifyPath(original); err == nil {
t.Fatal("VerifyPath succeeded after the lexical path changed")
}
}
func TestCreateDirectoryNoFollowPinsCreatedTaskRootBeforeMarkerWrite(t *testing.T) {
parent := t.TempDir()
tasksBase := filepath.Join(parent, "tasks")
taskRoot := filepath.Join(tasksBase, "task-one")
outside := filepath.Join(parent, "outside")
if err := os.MkdirAll(outside, 0o755); err != nil {
t.Fatalf("mkdir outside: %v", err)
}
handle, err := CreateDirectoryNoFollow(tasksBase, taskRoot, 0o755)
if err != nil {
t.Fatalf("create task root: %v", err)
}
t.Cleanup(func() { _ = handle.Close() })
archived := tasksBase + ".archived"
if err := os.Rename(tasksBase, archived); err != nil {
t.Fatalf("rename tasks base: %v", err)
}
if err := os.Symlink(outside, tasksBase); err != nil {
t.Skipf("symlink creation is unavailable: %v", err)
}
if err := handle.VerifyPath(taskRoot); err == nil {
t.Fatal("VerifyPath succeeded after the task root was replaced")
}
if err := WriteOwnershipMarkerNoFollow(handle, OwnershipMarker{
TaskID: "task-one",
TaskDirName: "task-one",
LayoutVersion: LayoutVersionSemantic,
}); err != nil {
t.Fatalf("write marker through opened task root: %v", err)
}
if _, err := os.Lstat(filepath.Join(outside, OwnershipMarkerFilename)); !os.IsNotExist(err) {
t.Fatalf("marker escaped into replacement target: %v", err)
}
}
@@ -0,0 +1,233 @@
//go:build !windows
package workspaces
import (
"errors"
"fmt"
"io"
"os"
"path/filepath"
"strings"
"sync"
"golang.org/x/sys/unix"
)
type unixDirectoryHandle struct {
rootFD int
parentFD int
targetFD int
once sync.Once
}
// OpenDirectoryNoFollow opens root and target with O_NOFOLLOW for every
// component. The returned handle remains attached to the original directory
// even if its lexical path is renamed or replaced later.
func OpenDirectoryNoFollow(root, target string) (DirectoryHandle, error) {
relative, err := dependencyRelativePath(root, target)
if err != nil {
return nil, err
}
rootFD, err := openDependencyDirectoryPath(root)
if err != nil {
return nil, err
}
parentFD, targetFD, err := openDependencyTarget(rootFD, relative)
if err != nil {
_ = unix.Close(rootFD)
return nil, err
}
return &unixDirectoryHandle{rootFD: rootFD, parentFD: parentFD, targetFD: targetFD}, nil
}
// CreateDirectoryNoFollow creates every missing component below root through
// directory descriptors. Existing symlinks and non-directories are rejected.
func CreateDirectoryNoFollow(root, target string, mode os.FileMode) (DirectoryHandle, error) {
relative, err := dependencyRelativePath(root, target)
if err != nil {
return nil, err
}
rootFD, err := openOrCreateDependencyDirectoryPath(root, mode)
if err != nil {
return nil, err
}
parentFD, targetFD, err := openOrCreateDependencyTarget(rootFD, relative, mode)
if err != nil {
_ = unix.Close(rootFD)
return nil, err
}
return &unixDirectoryHandle{rootFD: rootFD, parentFD: parentFD, targetFD: targetFD}, nil
}
func (h *unixDirectoryHandle) Close() error {
var closeErr error
h.once.Do(func() {
if h.targetFD >= 0 {
if err := unix.Close(h.targetFD); err != nil {
closeErr = err
}
}
if h.parentFD >= 0 && h.parentFD != h.rootFD {
if err := unix.Close(h.parentFD); err != nil && closeErr == nil {
closeErr = err
}
}
if h.rootFD >= 0 {
if err := unix.Close(h.rootFD); err != nil && closeErr == nil {
closeErr = err
}
}
})
return closeErr
}
func (h *unixDirectoryHandle) VerifyPath(path string) error {
if h == nil || h.targetFD < 0 {
return errors.New("directory handle is closed")
}
pathInfo, err := os.Lstat(path)
if err != nil {
return err
}
dup, err := unix.Dup(h.targetFD)
if err != nil {
return fmt.Errorf("duplicate directory handle: %w", err)
}
file := os.NewFile(uintptr(dup), "worktree-directory")
if file == nil {
_ = unix.Close(dup)
return errors.New("create directory file from handle")
}
handleInfo, statErr := file.Stat()
_ = file.Close()
if statErr != nil {
return fmt.Errorf("stat directory handle: %w", statErr)
}
if !os.SameFile(pathInfo, handleInfo) {
return fmt.Errorf("directory path changed: %s", path)
}
return nil
}
func (h *unixDirectoryHandle) IsValidWorktree() bool {
content, err := h.ReadFile(".git")
return err == nil && strings.HasPrefix(string(content), "gitdir:")
}
func (h *unixDirectoryHandle) ReadFile(name string) ([]byte, error) {
if h == nil || h.targetFD < 0 {
return nil, errors.New("directory handle is closed")
}
if err := validateDirectoryEntryName(name); err != nil {
return nil, err
}
fd, err := unix.Openat(h.targetFD, name, unix.O_RDONLY|unix.O_CLOEXEC|unix.O_NOFOLLOW, 0)
if err != nil {
return nil, err
}
file := os.NewFile(uintptr(fd), filepath.Join("worktree-directory", name))
if file == nil {
_ = unix.Close(fd)
return nil, errors.New("create file from directory entry handle")
}
info, statErr := file.Stat()
if statErr != nil {
_ = file.Close()
return nil, statErr
}
if !info.Mode().IsRegular() {
_ = file.Close()
return nil, fmt.Errorf("directory entry is not a regular file: %s", name)
}
content, readErr := io.ReadAll(file)
_ = file.Close()
return content, readErr
}
func (h *unixDirectoryHandle) WriteFile(name string, data []byte, mode os.FileMode) error {
if h == nil || h.targetFD < 0 {
return errors.New("directory handle is closed")
}
if err := validateDirectoryEntryName(name); err != nil {
return err
}
fd, err := unix.Openat(h.targetFD, name,
unix.O_WRONLY|unix.O_CREAT|unix.O_TRUNC|unix.O_CLOEXEC|unix.O_NOFOLLOW,
uint32(mode.Perm()))
if err != nil {
return err
}
file := os.NewFile(uintptr(fd), filepath.Join("worktree-directory", name))
if file == nil {
_ = unix.Close(fd)
return errors.New("create writable file from directory entry handle")
}
if err := unix.Fchmod(fd, uint32(mode.Perm())); err != nil {
_ = file.Close()
return err
}
if _, err := file.Write(data); err != nil {
_ = file.Close()
return err
}
return file.Close()
}
func openOrCreateDependencyDirectoryPath(path string, mode os.FileMode) (int, error) {
if !filepath.IsAbs(path) {
return -1, fmt.Errorf("dependency workspace root must be absolute")
}
fd, err := unix.Open(string(filepath.Separator), dependencyDirectoryOpenFlags, 0)
if err != nil {
return -1, err
}
for _, part := range dependencyAbsolutePathComponents(path) {
next, openErr := unix.Openat(fd, part, dependencyDirectoryOpenFlags, 0)
if errors.Is(openErr, unix.ENOENT) {
if mkdirErr := unix.Mkdirat(fd, part, uint32(mode.Perm())); mkdirErr != nil && !errors.Is(mkdirErr, unix.EEXIST) {
_ = unix.Close(fd)
return -1, mkdirErr
}
next, openErr = unix.Openat(fd, part, dependencyDirectoryOpenFlags, 0)
}
if openErr != nil {
_ = unix.Close(fd)
return -1, openErr
}
_ = unix.Close(fd)
fd = next
}
return fd, nil
}
func openOrCreateDependencyTarget(rootFD int, relative string, mode os.FileMode) (parentFD, targetFD int, err error) {
parts := strings.Split(filepath.ToSlash(relative), "/")
parentFD = rootFD
for index, part := range parts {
next, openErr := unix.Openat(parentFD, part, dependencyDirectoryOpenFlags, 0)
if errors.Is(openErr, unix.ENOENT) {
if mkdirErr := unix.Mkdirat(parentFD, part, uint32(mode.Perm())); mkdirErr != nil && !errors.Is(mkdirErr, unix.EEXIST) {
if parentFD != rootFD {
_ = unix.Close(parentFD)
}
return -1, -1, mkdirErr
}
next, openErr = unix.Openat(parentFD, part, dependencyDirectoryOpenFlags, 0)
}
if openErr != nil {
if parentFD != rootFD {
_ = unix.Close(parentFD)
}
return -1, -1, openErr
}
if index == len(parts)-1 {
return parentFD, next, nil
}
if parentFD != rootFD {
_ = unix.Close(parentFD)
}
parentFD = next
}
return -1, -1, fmt.Errorf("dependency target is empty")
}
@@ -0,0 +1,268 @@
//go:build windows
package workspaces
import (
"errors"
"fmt"
"io"
"os"
"path/filepath"
"strings"
"sync"
"golang.org/x/sys/windows"
)
type windowsDirectoryHandle struct {
rootHandle windows.Handle
parentHandle windows.Handle
targetHandle windows.Handle
once sync.Once
}
// OpenDirectoryNoFollow opens root and target with OBJ_DONT_REPARSE and
// FILE_OPEN_REPARSE_POINT for every component.
func OpenDirectoryNoFollow(root, target string) (DirectoryHandle, error) {
relative, err := dependencyRelativePath(root, target)
if err != nil {
return nil, err
}
rootHandle, err := openWindowsDependencyDirectoryPath(root)
if err != nil {
return nil, err
}
parentHandle, targetHandle, err := openWindowsDependencyTarget(rootHandle, relative)
if err != nil {
_ = windows.CloseHandle(rootHandle)
return nil, err
}
return &windowsDirectoryHandle{
rootHandle: rootHandle, parentHandle: parentHandle, targetHandle: targetHandle,
}, nil
}
// CreateDirectoryNoFollow creates every missing component below root through
// native directory handles. Reparse points and non-directories are rejected.
func CreateDirectoryNoFollow(root, target string, _ os.FileMode) (DirectoryHandle, error) {
relative, err := dependencyRelativePath(root, target)
if err != nil {
return nil, err
}
rootHandle, err := openOrCreateWindowsDependencyDirectoryPath(root)
if err != nil {
return nil, err
}
parentHandle, targetHandle, err := openOrCreateWindowsDependencyTarget(rootHandle, relative)
if err != nil {
_ = windows.CloseHandle(rootHandle)
return nil, err
}
return &windowsDirectoryHandle{
rootHandle: rootHandle, parentHandle: parentHandle, targetHandle: targetHandle,
}, nil
}
func (h *windowsDirectoryHandle) Close() error {
var closeErr error
h.once.Do(func() {
if h.targetHandle != 0 {
if err := windows.CloseHandle(h.targetHandle); err != nil {
closeErr = err
}
}
if h.parentHandle != 0 && h.parentHandle != h.rootHandle {
if err := windows.CloseHandle(h.parentHandle); err != nil && closeErr == nil {
closeErr = err
}
}
if h.rootHandle != 0 {
if err := windows.CloseHandle(h.rootHandle); err != nil && closeErr == nil {
closeErr = err
}
}
})
return closeErr
}
func (h *windowsDirectoryHandle) VerifyPath(path string) error {
if h == nil || h.targetHandle == 0 {
return errors.New("directory handle is closed")
}
pathInfo, err := os.Lstat(path)
if err != nil {
return err
}
dup, err := duplicateWindowsDependencyHandle(h.targetHandle)
if err != nil {
return fmt.Errorf("duplicate directory handle: %w", err)
}
file := os.NewFile(uintptr(dup), "worktree-directory")
if file == nil {
_ = windows.CloseHandle(dup)
return errors.New("create directory file from handle")
}
handleInfo, statErr := file.Stat()
_ = file.Close()
if statErr != nil {
return fmt.Errorf("stat directory handle: %w", statErr)
}
if !os.SameFile(pathInfo, handleInfo) {
return fmt.Errorf("directory path changed: %s", path)
}
return nil
}
func (h *windowsDirectoryHandle) IsValidWorktree() bool {
content, err := h.ReadFile(".git")
return err == nil && strings.HasPrefix(string(content), "gitdir:")
}
func (h *windowsDirectoryHandle) ReadFile(name string) ([]byte, error) {
if h == nil || h.targetHandle == 0 {
return nil, errors.New("directory handle is closed")
}
if err := validateDirectoryEntryName(name); err != nil {
return nil, err
}
handle, err := openWindowsDependencyEntryRelative(h.targetHandle, name)
if err != nil {
return nil, err
}
if reparse, checkErr := windowsDependencyHandleIsReparsePoint(handle); checkErr != nil {
_ = windows.CloseHandle(handle)
return nil, checkErr
} else if reparse {
_ = windows.CloseHandle(handle)
return nil, fmt.Errorf("directory entry is a reparse point: %s", name)
}
file := os.NewFile(uintptr(handle), filepath.Join("worktree-directory", name))
if file == nil {
_ = windows.CloseHandle(handle)
return nil, errors.New("create file from directory entry handle")
}
info, statErr := file.Stat()
if statErr != nil {
_ = file.Close()
return nil, statErr
}
if !info.Mode().IsRegular() {
_ = file.Close()
return nil, fmt.Errorf("directory entry is not a regular file: %s", name)
}
content, readErr := io.ReadAll(file)
_ = file.Close()
return content, readErr
}
func (h *windowsDirectoryHandle) WriteFile(name string, data []byte, _ os.FileMode) error {
if h == nil || h.targetHandle == 0 {
return errors.New("directory handle is closed")
}
if err := validateDirectoryEntryName(name); err != nil {
return err
}
handle, err := openWindowsDependencyHandleWithDisposition(
h.targetHandle, name, windows.FILE_OVERWRITE_IF, windows.FILE_NON_DIRECTORY_FILE,
)
if err != nil {
return err
}
file := os.NewFile(uintptr(handle), filepath.Join("worktree-directory", name))
if file == nil {
_ = windows.CloseHandle(handle)
return errors.New("create writable file from directory entry handle")
}
if _, err := file.Write(data); err != nil {
_ = file.Close()
return err
}
return file.Close()
}
func openOrCreateWindowsDependencyDirectoryPath(path string) (windows.Handle, error) {
root, parts, err := windowsDependencyPathParts(path)
if err != nil {
return 0, err
}
rootHandle, err := openWindowsDependencyHandle(0, windowsDependencyNTPath(root), windows.FILE_DIRECTORY_FILE)
if err != nil {
return 0, err
}
for _, part := range parts {
next, openErr := openWindowsDependencyHandleWithDisposition(
rootHandle, part, windows.FILE_OPEN_IF, windows.FILE_DIRECTORY_FILE,
)
if openErr != nil {
_ = windows.CloseHandle(rootHandle)
return 0, openErr
}
if reparse, checkErr := windowsDependencyHandleIsReparsePoint(next); checkErr != nil {
_ = windows.CloseHandle(next)
_ = windows.CloseHandle(rootHandle)
return 0, checkErr
} else if reparse {
_ = windows.CloseHandle(next)
_ = windows.CloseHandle(rootHandle)
return 0, fmt.Errorf("dependency path is a reparse point")
}
_ = windows.CloseHandle(rootHandle)
rootHandle = next
}
return rootHandle, nil
}
func openOrCreateWindowsDependencyTarget(rootHandle windows.Handle, relative string) (parent, target windows.Handle, err error) {
parts := strings.FieldsFunc(filepath.Clean(relative), func(r rune) bool { return r == '\\' || r == '/' })
parent = rootHandle
for index, part := range parts {
next, openErr := openWindowsDependencyHandleWithDisposition(
parent, part, windows.FILE_OPEN_IF, windows.FILE_DIRECTORY_FILE,
)
if openErr != nil {
if parent != rootHandle {
_ = windows.CloseHandle(parent)
}
return 0, 0, openErr
}
if reparse, checkErr := windowsDependencyHandleIsReparsePoint(next); checkErr != nil {
_ = windows.CloseHandle(next)
if parent != rootHandle {
_ = windows.CloseHandle(parent)
}
return 0, 0, checkErr
} else if reparse {
_ = windows.CloseHandle(next)
if parent != rootHandle {
_ = windows.CloseHandle(parent)
}
return 0, 0, fmt.Errorf("dependency path is a reparse point")
}
if index == len(parts)-1 {
return parent, next, nil
}
if parent != rootHandle {
_ = windows.CloseHandle(parent)
}
parent = next
}
return 0, 0, fmt.Errorf("dependency target is empty")
}
func windowsDependencyPathParts(path string) (string, []string, error) {
if !filepath.IsAbs(path) {
return "", nil, fmt.Errorf("dependency workspace root must be absolute")
}
clean := filepath.Clean(path)
volume := filepath.VolumeName(clean)
if volume == "" {
return "", nil, fmt.Errorf("dependency workspace root has no volume")
}
remainder := strings.TrimPrefix(clean, volume)
remainder = strings.TrimLeft(remainder, `\`+`/`)
root := volume + `\`
if remainder == "" {
return root, nil, nil
}
return root, strings.FieldsFunc(remainder, func(r rune) bool { return r == '\\' || r == '/' }), nil
}
+6
View File
@@ -93,6 +93,12 @@ var (
// find a valid canonical worktree. Callers must surface this as a workspace
// reuse failure; they must never fall through to git worktree add.
ErrReuseWorktreeUnavailable = errors.New("required worktree is unavailable for reuse")
// ErrWorktreePathOwnedByAnotherTask is returned when a persisted worktree
// record points at a task root whose ownership marker belongs to a
// different task. Reusing or recreating that record could attach to or
// delete the other task's live checkout.
ErrWorktreePathOwnedByAnotherTask = errors.New("worktree path is owned by another task")
)
// containsAuthFailure checks if git output indicates an authentication failure.
@@ -198,8 +198,17 @@ func (m *Manager) reuseRequiredWorktree(ctx context.Context, req CreateRequest)
if wt == nil || wt.Status != StatusActive ||
wt.RepositoryID != req.RepositoryID ||
wt.TaskEnvironmentID != req.TaskEnvironmentID ||
(requestedBranchSlug != "" && SanitizeBranchSlug(wt.BranchSlug) != requestedBranchSlug) ||
!m.IsValid(wt.Path) {
(requestedBranchSlug != "" && SanitizeBranchSlug(wt.BranchSlug) != requestedBranchSlug) {
return nil, ErrReuseWorktreeUnavailable
}
handle, valid, err := m.openReusableWorktreePath(wt.Path, wt)
if err != nil {
return nil, err
}
if handle != nil {
defer func() { _ = handle.Close() }()
}
if !valid {
return nil, ErrReuseWorktreeUnavailable
}
return wt, nil
@@ -220,7 +229,14 @@ func (m *Manager) tryReuseExisting(ctx context.Context, req CreateRequest) (*Wor
if req.SessionID != "" {
existing, err := m.GetBySessionAndRepo(ctx, req.SessionID, req.RepositoryID, reuseSlug)
if err == nil && existing != nil {
if m.IsValid(existing.Path) {
handle, valid, err := m.openReusableWorktreePath(existing.Path, existing)
if err != nil {
return nil, true, err
}
if handle != nil {
defer func() { _ = handle.Close() }()
}
if valid {
if err := m.validateReusableContribution(ctx, req, existing); err != nil {
return nil, true, err
}
@@ -246,7 +262,14 @@ func (m *Manager) tryReuseExisting(ctx context.Context, req CreateRequest) (*Wor
if req.WorktreeID != "" {
existing, err := m.GetByID(ctx, req.WorktreeID)
if err == nil && existing != nil {
if m.IsValid(existing.Path) {
handle, valid, err := m.openReusableWorktreePath(existing.Path, existing)
if err != nil {
return nil, true, err
}
if handle != nil {
defer func() { _ = handle.Close() }()
}
if valid {
if err := m.validateReusableContribution(ctx, req, existing); err != nil {
return nil, true, err
}
@@ -274,6 +297,147 @@ func (m *Manager) tryReuseExisting(ctx context.Context, req CreateRequest) (*Wor
return nil, false, nil
}
// validateWorktreePathSafe opens a persisted worktree path through no-follow
// directory handles. The handle is returned to the caller so the same
// directory identity is used for ownership and reuse decisions. Missing paths
// remain eligible for the normal recreate flow, while symlinked components
// fail closed.
func (m *Manager) validateWorktreePathSafe(worktreePath string) (storageworkspaces.DirectoryHandle, error) {
if worktreePath == "" {
return nil, nil
}
tasksBase, err := m.config.ExpandedTasksBasePath()
if err != nil {
return nil, fmt.Errorf("resolve tasks base path: %w", err)
}
tasksBase = filepath.Clean(tasksBase)
// Paths outside the tasks base use the legacy flat layout. They do not have
// a task-root ownership marker, but still get a no-follow handle so reuse
// cannot follow a replacement link while checking the checkout.
relativePath, err := filepath.Rel(tasksBase, worktreePath)
if err != nil || relativePath == "." || relativePath == ".." || strings.HasPrefix(relativePath, ".."+string(filepath.Separator)) {
// Legacy paths are not below tasksBase, but they still get a pinned
// no-follow handle so IsValid cannot be redirected by a symlink race.
return m.openNoFollowWorktreePath(worktreePath)
}
if _, err := os.Lstat(tasksBase); errors.Is(err, os.ErrNotExist) {
// A removed task base has no components to follow. Normal reuse will
// recreate the persisted managed root before adding the worktree;
// attach-only reuse still rejects the missing checkout via IsValid.
return nil, nil
} else if err != nil {
return nil, fmt.Errorf("inspect tasks base path: %w", err)
}
return m.openNoFollowWorktreePath(worktreePath)
}
func (m *Manager) openNoFollowWorktreePath(worktreePath string) (storageworkspaces.DirectoryHandle, error) {
cleanPath := filepath.Clean(worktreePath)
if !filepath.IsAbs(cleanPath) {
return nil, nil
}
handle, err := storageworkspaces.OpenDirectoryNoFollow(filepath.Dir(cleanPath), cleanPath)
if err != nil {
if os.IsNotExist(err) {
return nil, nil
}
return nil, fmt.Errorf("unsafe worktree path: %w", err)
}
if err := handle.VerifyPath(cleanPath); err != nil {
_ = handle.Close()
return nil, fmt.Errorf("worktree path changed during validation: %w", err)
}
return handle, nil
}
func (m *Manager) openReusableWorktreePath(worktreePath string, wt *Worktree) (storageworkspaces.DirectoryHandle, bool, error) {
handle, err := m.validateWorktreePathSafe(worktreePath)
if err != nil {
return nil, false, err
}
if err := m.validateExistingWorktreePathOwner(worktreePath, wt); err != nil {
if handle != nil {
_ = handle.Close()
}
return nil, false, err
}
if handle == nil || !handle.IsValidWorktree() {
if handle != nil {
_ = handle.Close()
}
return nil, false, nil
}
if err := handle.VerifyPath(filepath.Clean(worktreePath)); err != nil {
_ = handle.Close()
return nil, false, fmt.Errorf("worktree path changed during reuse validation: %w", err)
}
return handle, true, nil
}
func worktreeOwnershipIdentity(wt *Worktree) (string, bool) {
if wt != nil && wt.TaskDirName != "" {
return wt.TaskDirName, true
}
if wt != nil {
// Older in-memory and persisted records may not project TaskDirName.
// Keep their existing ownership check until they are refreshed.
return wt.TaskID, false
}
return "", false
}
// validateExistingWorktreePathOwner rejects a stale record whose stored path
// now lies under another task's marked root. It deliberately runs before both
// reuse and recreate: recreate removes the recorded path before adding a new
// worktree, so checking only the reusable path could delete a live checkout.
// Paths outside Kandev's task root and legacy unmarked task roots remain
// eligible for their existing compatibility behavior.
func (m *Manager) validateExistingWorktreePathOwner(worktreePath string, wt *Worktree) error {
if worktreePath == "" {
return nil
}
tasksBase, err := m.config.ExpandedTasksBasePath()
if err != nil {
return fmt.Errorf("resolve tasks base path: %w", err)
}
// Normalize: ExpandedTasksBasePath returns the configured value verbatim
// (incl. trailing slashes or doubled separators), while filepath.Dir
// always yields a cleaned form. Without this, the path computation below
// may miscompute the task-root segment.
tasksBase = filepath.Clean(tasksBase)
relativePath, err := filepath.Rel(tasksBase, worktreePath)
if err != nil || relativePath == "." || relativePath == ".." || strings.HasPrefix(relativePath, ".."+string(filepath.Separator)) {
return nil
}
recordIdentity, stableTaskDirName := worktreeOwnershipIdentity(wt)
if recordIdentity == "" {
return nil
}
// The task-root ownership marker always lives at the first path segment
// below tasksBase (i.e. <tasksBase>/<taskDirName>/). Walking all levels
// up from worktreePath is wrong: a descendant repository that legitimately
// contains its own .kandev-workspace.json (e.g. a nested project) would be
// mistaken for the task-root marker. Derive the task root from the
// relative path and inspect only that one directory.
taskRoot := filepath.Join(tasksBase, strings.SplitN(relativePath, string(filepath.Separator), 2)[0])
owner, found, err := storageworkspaces.ReadOwnershipMarker(taskRoot)
if err != nil {
return fmt.Errorf("inspect worktree task root ownership: %w", err)
}
// Validate against the environment's stable task directory name, not the
// mutable task ID. Shared environments can transfer ownership while their
// physical task root and marker remain unchanged. Older records without the
// projected directory name retain the task-ID compatibility check.
ownerMatches := owner.TaskDirName == recordIdentity
if !stableTaskDirName {
ownerMatches = owner.TaskID == recordIdentity
}
if found && !ownerMatches {
return fmt.Errorf("%w: path %q belongs to task %q", ErrWorktreePathOwnedByAnotherTask, worktreePath, owner.TaskID)
}
return nil
}
func (m *Manager) validateReusableContribution(ctx context.Context, req CreateRequest, existing *Worktree) error {
if req.RemoteContribution == nil {
return nil
@@ -1410,6 +1574,75 @@ func (m *Manager) copyConfiguredFiles(ctx context.Context, req CreateRequest, wt
wt.CopyFilesWarnings = warnings
}
// removeRecreatedWorktreePath deletes only a managed worktree path through
// no-follow directory handles. Legacy paths outside tasksBase retain their
// existing behavior because they are not controlled by the managed root.
func (m *Manager) removeRecreatedWorktreePath(ctx context.Context, worktreePath string) error {
tasksBase, err := m.config.ExpandedTasksBasePath()
if err != nil {
return fmt.Errorf("resolve tasks base path for recreate removal: %w", err)
}
tasksBase = filepath.Clean(tasksBase)
relativePath, err := filepath.Rel(tasksBase, worktreePath)
if err != nil || relativePath == "." || relativePath == ".." || strings.HasPrefix(relativePath, ".."+string(filepath.Separator)) {
if err := os.RemoveAll(worktreePath); err != nil {
return fmt.Errorf("remove existing worktree path: %w", err)
}
return nil
}
if err := storageworkspaces.RemoveDirectoryNoFollow(ctx, tasksBase, worktreePath); err != nil && !errors.Is(err, os.ErrNotExist) {
return fmt.Errorf("remove existing worktree path without following links: %w", err)
}
return nil
}
// restoreMissingTasksBaseForRecreate rebuilds the task-root directory only when
// the configured managed base disappeared. The persisted task directory name
// fixes the root identity, while a fresh symlink validation protects the newly
// created path before Git is allowed to populate it.
func (m *Manager) restoreMissingTasksBaseForRecreate(worktreePath string, existing *Worktree) error {
tasksBase, err := m.config.ExpandedTasksBasePath()
if err != nil {
return fmt.Errorf("resolve tasks base path for recreate: %w", err)
}
tasksBase = filepath.Clean(tasksBase)
relativePath, err := filepath.Rel(tasksBase, worktreePath)
if err != nil || relativePath == "." || relativePath == ".." || strings.HasPrefix(relativePath, ".."+string(filepath.Separator)) {
return nil
}
if _, err := os.Lstat(tasksBase); !errors.Is(err, os.ErrNotExist) {
if err != nil {
return fmt.Errorf("inspect tasks base path for recreate: %w", err)
}
return nil
}
taskDirName := strings.SplitN(relativePath, string(filepath.Separator), 2)[0]
if existing.TaskDirName != "" && existing.TaskDirName != taskDirName {
return fmt.Errorf("cannot recreate worktree: persisted task root %q does not match path root %q", existing.TaskDirName, taskDirName)
}
taskRoot := filepath.Join(tasksBase, taskDirName)
// Build the missing base and task root through no-follow directory handles.
// This keeps a concurrent replacement of tasksBase from redirecting marker
// creation into an external directory.
taskRootHandle, err := storageworkspaces.CreateDirectoryNoFollow(tasksBase, taskRoot, 0o755)
if err != nil {
return fmt.Errorf("recreate missing tasks base: %w", err)
}
defer func() { _ = taskRootHandle.Close() }()
if err := taskRootHandle.VerifyPath(taskRoot); err != nil {
return fmt.Errorf("recreated task root changed during recovery: %w", err)
}
markerTaskDirName := taskDirName
if err := storageworkspaces.WriteOwnershipMarkerNoFollow(taskRootHandle, storageworkspaces.OwnershipMarker{
TaskID: existing.TaskID, TaskDirName: markerTaskDirName,
LayoutVersion: storageworkspaces.LayoutVersionSemantic,
}); err != nil {
return fmt.Errorf("mark recreated task directory ownership: %w", err)
}
return nil
}
// recreate recreates a worktree from stored metadata.
func (m *Manager) recreate(ctx context.Context, existing *Worktree, req CreateRequest) (*Worktree, error) {
if err := m.refreshRepositoryForMaterialization(ctx, &req); err != nil {
@@ -1425,10 +1658,12 @@ func (m *Manager) recreate(ctx context.Context, existing *Worktree, req CreateRe
}
}
// Clean up existing directory if present
// Clean up the recorded directory through no-follow descriptors. Reuse
// validation can race with a local rename, so path-based RemoveAll here
// could otherwise be redirected into another task root.
if existing.Path != "" {
if err := os.RemoveAll(existing.Path); err != nil {
m.logger.Debug("failed to remove existing worktree path", zap.Error(err))
if err := m.removeRecreatedWorktreePath(ctx, existing.Path); err != nil {
return nil, err
}
}
@@ -1453,6 +1688,9 @@ func (m *Manager) recreate(ctx context.Context, existing *Worktree, req CreateRe
if worktreePath == "" {
return nil, fmt.Errorf("cannot recreate worktree: existing record has no path")
}
if err := m.restoreMissingTasksBaseForRecreate(worktreePath, existing); err != nil {
return nil, err
}
// Archive deletes the local branch (removeWorktree runs `git branch -D`),
// so a recreate after unarchive must restore it first. fetchBranchToLocal
@@ -115,6 +115,52 @@ esac
}
}
// TestRecreate_RejectsSymlinkedAncestorBeforeRemoval ensures recreate does not
// delete through an ancestor that changed into a symlink after earlier reuse
// validation.
func TestRecreate_RejectsSymlinkedAncestorBeforeRemoval(t *testing.T) {
repoPath := initGitRepoWithRemote(t)
cfg := newTestConfig(t)
mgr, err := NewManager(cfg, newMockStore(), newTestLogger())
if err != nil {
t.Fatalf("NewManager() error = %v", err)
}
outside := t.TempDir()
liveWorktree := filepath.Join(outside, "repo-one")
if err := os.MkdirAll(liveWorktree, 0o755); err != nil {
t.Fatalf("mkdir live worktree: %v", err)
}
canary := filepath.Join(liveWorktree, "must-survive")
if err := os.WriteFile(canary, []byte("live task data"), 0o600); err != nil {
t.Fatalf("write canary: %v", err)
}
taskRoot := filepath.Join(cfg.TasksBasePath, "task-one")
if err := os.Symlink(outside, taskRoot); err != nil {
t.Fatalf("replace task root with symlink: %v", err)
}
_, err = mgr.recreate(context.Background(), &Worktree{
ID: "wt-symlink-race",
TaskID: "task-one",
TaskDirName: "task-one",
RepositoryID: "repo-one",
Path: filepath.Join(taskRoot, "repo-one"),
Branch: "feature/pr-branch",
Status: StatusActive,
}, CreateRequest{
TaskID: "task-one",
RepositoryID: "repo-one",
RepositoryPath: repoPath,
})
if err == nil {
t.Fatal("recreate() error = nil, want symlinked ancestor rejection")
}
if _, statErr := os.Stat(canary); statErr != nil {
t.Fatalf("recreate() touched live worktree through symlink: %v", statErr)
}
}
// TestRecreate_BranchGoneEverywhereReturnsUnrecoverable pins the degraded
// path: local branch deleted AND the branch never made it to origin (or was
// deleted there too). recreate must return ErrBranchUnrecoverable so callers
@@ -7,6 +7,8 @@ import (
"path/filepath"
"strings"
"testing"
storageworkspaces "github.com/kandev/kandev/internal/system/storage/workspaces"
)
func TestCreate_ReuseRequiredRejectsMissingCanonicalWorktree(t *testing.T) {
@@ -26,6 +28,106 @@ func TestCreate_ReuseRequiredRejectsMissingCanonicalWorktree(t *testing.T) {
}
}
// TestCreate_RecreatesPersistedWorktreeWhenTasksBaseWasRemoved verifies that
// normal reuse treats a removed managed tasks root as a missing worktree. The
// persisted task-root identity still identifies the original safe slot, so the
// lifecycle may recreate it rather than treating the absent validation anchor
// as a symlink-safety failure.
func TestCreate_RecreatesPersistedWorktreeWhenTasksBaseWasRemoved(t *testing.T) {
repoPath := initGitRepoWithRemote(t)
cfg := newTestConfig(t)
taskDirName := "task-one_recovery"
worktreePath := filepath.Join(cfg.TasksBasePath, taskDirName, "repo-one")
if err := os.MkdirAll(filepath.Dir(worktreePath), 0o755); err != nil {
t.Fatalf("mkdir task directory: %v", err)
}
runGit(t, repoPath, "worktree", "add", "-b", "recover-missing-tasks-base", worktreePath, "main")
store := newMockStore()
store.worktrees["canonical-worktree"] = &Worktree{
ID: "canonical-worktree",
TaskID: "task-one",
TaskDirName: taskDirName,
SessionID: "session-one",
TaskEnvironmentID: "environment-one",
RepositoryID: "repository-one",
Path: worktreePath,
Branch: "recover-missing-tasks-base",
Status: StatusActive,
}
mgr, err := NewManager(cfg, store, newTestLogger())
if err != nil {
t.Fatalf("NewManager() error = %v", err)
}
if err := os.RemoveAll(cfg.TasksBasePath); err != nil {
t.Fatalf("remove tasks base: %v", err)
}
got, err := mgr.Create(context.Background(), CreateRequest{
TaskID: "task-one",
SessionID: "session-one",
TaskEnvironmentID: "environment-one",
RepositoryID: "repository-one",
RepositoryPath: repoPath,
BaseBranch: "main",
})
if err != nil {
t.Fatalf("Create() error = %v", err)
}
if got.Path != worktreePath {
t.Fatalf("Create() path = %q, want %q", got.Path, worktreePath)
}
if !mgr.IsValid(worktreePath) {
t.Fatalf("Create() did not recreate a valid worktree at %q", worktreePath)
}
owner, found, err := storageworkspaces.ReadOwnershipMarker(filepath.Dir(worktreePath))
if err != nil {
t.Fatalf("read recreated ownership marker: %v", err)
}
if !found || owner.TaskDirName != taskDirName || owner.TaskID != "task-one" {
t.Fatalf("recreated ownership marker = %#v, found=%v", owner, found)
}
}
// TestCreate_ReuseRequiredRejectsRemovedTasksBase confirms attach-only reuse
// remains fail-closed: it must not recreate a missing canonical checkout.
func TestCreate_ReuseRequiredRejectsRemovedTasksBase(t *testing.T) {
cfg := newTestConfig(t)
worktreePath := filepath.Join(cfg.TasksBasePath, "task-one_recovery", "repo-one")
store := newMockStore()
store.worktrees["canonical-worktree"] = &Worktree{
ID: "canonical-worktree",
TaskID: "task-one",
TaskDirName: "task-one_recovery",
TaskEnvironmentID: "environment-one",
RepositoryID: "repository-one",
Path: worktreePath,
Branch: "recover-missing-tasks-base",
Status: StatusActive,
}
mgr, err := NewManager(cfg, store, newTestLogger())
if err != nil {
t.Fatalf("NewManager() error = %v", err)
}
if err := os.RemoveAll(cfg.TasksBasePath); err != nil {
t.Fatalf("remove tasks base: %v", err)
}
_, err = mgr.Create(context.Background(), CreateRequest{
TaskID: "task-one",
TaskEnvironmentID: "environment-one",
RepositoryID: "repository-one",
RepositoryPath: t.TempDir(),
BaseBranch: "main",
WorktreeID: "canonical-worktree",
ReuseRequired: true,
})
if !errors.Is(err, ErrReuseWorktreeUnavailable) {
t.Fatalf("Create() error = %v, want ErrReuseWorktreeUnavailable", err)
}
}
func TestCreate_ReuseRequiredReturnsCanonicalWorktreeWithoutChangingGitState(t *testing.T) {
repoPath := initGitRepoWithRemote(t)
worktreePath := filepath.Join(t.TempDir(), "canonical")
@@ -138,7 +240,22 @@ func TestCreate_ReuseRequiredRejectsCanonicalWorktreeFromAnotherBranch(t *testin
func TestCreate_ReuseRequiredAllowsAuthorizedEnvironmentFromAnotherTask(t *testing.T) {
repoPath := initGitRepoWithRemote(t)
worktreePath := filepath.Join(t.TempDir(), "canonical")
cfg := newTestConfig(t)
tasksBase := cfg.TasksBasePath
ownerTaskDir := filepath.Join(tasksBase, "owner-task_dir")
if err := os.MkdirAll(ownerTaskDir, 0755); err != nil {
t.Fatalf("mkdir owner task dir: %v", err)
}
if err := storageworkspaces.WriteOwnershipMarker(ownerTaskDir, storageworkspaces.OwnershipMarker{
TaskID: "owner-task",
TaskDirName: "owner-task_dir",
LayoutVersion: storageworkspaces.LayoutVersionSemantic,
}); err != nil {
t.Fatalf("write owner ownership marker: %v", err)
}
worktreePath := filepath.Join(ownerTaskDir, "repo-one")
runGit(t, repoPath, "worktree", "add", "-b", "shared-environment", worktreePath, "main")
store := newMockStore()
store.worktrees["canonical-worktree"] = &Worktree{
@@ -150,7 +267,7 @@ func TestCreate_ReuseRequiredAllowsAuthorizedEnvironmentFromAnotherTask(t *testi
Branch: "shared-environment",
Status: StatusActive,
}
mgr, err := NewManager(newTestConfig(t), store, newTestLogger())
mgr, err := NewManager(cfg, store, newTestLogger())
if err != nil {
t.Fatalf("NewManager() error = %v", err)
}
@@ -172,3 +289,261 @@ func TestCreate_ReuseRequiredAllowsAuthorizedEnvironmentFromAnotherTask(t *testi
t.Fatalf("Create() worktree ID = %q, want canonical-worktree", got.ID)
}
}
// TestCreate_ReuseRequiredRejectsStaleWorktreePathOwnedByAnotherTask
// confirms that the attach-only ReuseRequired path also validates the
// worktree path's ownership marker, rejecting a record whose on-disk
// path belongs to another live task.
func TestCreate_ReuseRequiredRejectsStaleWorktreePathOwnedByAnotherTask(t *testing.T) {
repoPath := initGitRepoWithRemote(t)
cfg := newTestConfig(t)
tasksBase := cfg.TasksBasePath
// Create the live task's directory and ownership marker.
liveTaskDir := filepath.Join(tasksBase, "live-task_reuse")
if err := os.MkdirAll(liveTaskDir, 0755); err != nil {
t.Fatalf("mkdir live task dir: %v", err)
}
if err := storageworkspaces.WriteOwnershipMarker(liveTaskDir, storageworkspaces.OwnershipMarker{
TaskID: "task-live-reuse",
TaskDirName: "live-task_reuse",
LayoutVersion: storageworkspaces.LayoutVersionSemantic,
}); err != nil {
t.Fatalf("write live ownership marker: %v", err)
}
// The worktree lives inside the live task's directory, as it would in
// production: <tasksBase>/<taskDirName>/<repoName>/
liveWorktreePath := filepath.Join(liveTaskDir, "my-repo")
if err := os.MkdirAll(liveWorktreePath, 0755); err != nil {
t.Fatalf("mkdir live worktree: %v", err)
}
runGit(t, repoPath, "worktree", "add", "-b", "feature/reuse-stale", liveWorktreePath, "main")
store := newMockStore()
store.worktrees["canonical-reuse"] = &Worktree{
ID: "canonical-reuse",
TaskID: "task-stale-reuse",
TaskEnvironmentID: "environment-1",
RepositoryID: "repository-1",
Path: liveWorktreePath,
Branch: "feature/reuse-stale",
Status: StatusActive,
}
mgr, err := NewManager(cfg, store, newTestLogger())
if err != nil {
t.Fatalf("NewManager() error = %v", err)
}
_, err = mgr.Create(context.Background(), CreateRequest{
TaskID: "task-attach-reuse",
SessionID: "session-attach",
TaskEnvironmentID: "environment-1",
RepositoryID: "repository-1",
RepositoryPath: repoPath,
BaseBranch: "main",
WorktreeID: "canonical-reuse",
ReuseRequired: true,
})
if !errors.Is(err, ErrWorktreePathOwnedByAnotherTask) {
t.Fatalf("Create() error = %v, want ErrWorktreePathOwnedByAnotherTask", err)
}
}
// TestCreate_ReuseRequiredRejectsSymlinkedWorktreePath confirms that the
// attach-only ReuseRequired path rejects a persisted worktree path whose
// existing components beneath tasksBase contain a symlink. A stale record
// whose final repo directory has been symlinked to another task's checkout
// must not follow the symlink via os.Stat/os.ReadFile inside IsValid.
func TestCreate_ReuseRequiredRejectsSymlinkedWorktreePath(t *testing.T) {
repoPath := initGitRepoWithRemote(t)
cfg := newTestConfig(t)
tasksBase := cfg.TasksBasePath
// Create two task directories under tasksBase.
liveTaskDir := filepath.Join(tasksBase, "live-task_sym")
if err := os.MkdirAll(liveTaskDir, 0755); err != nil {
t.Fatalf("mkdir live task dir: %v", err)
}
if err := storageworkspaces.WriteOwnershipMarker(liveTaskDir, storageworkspaces.OwnershipMarker{
TaskID: "task-live-sym",
TaskDirName: "live-task_sym",
LayoutVersion: storageworkspaces.LayoutVersionSemantic,
}); err != nil {
t.Fatalf("write live ownership marker: %v", err)
}
liveWorktreePath := filepath.Join(liveTaskDir, "my-repo")
runGit(t, repoPath, "worktree", "add", "-b", "feature/live", liveWorktreePath, "main")
// Create a stale task directory whose "repo" entry is a symlink to the
// live checkout. A clean redo that creates the stale worktree before the
// live one and then replaces the final component with a symlink would be
// more realistic, but a direct symlink is deterministic and exercises the
// same code path: the record's TaskID !== the live marker's TaskID.
staleTaskDir := filepath.Join(tasksBase, "stale-task_sym")
if err := os.MkdirAll(staleTaskDir, 0755); err != nil {
t.Fatalf("mkdir stale task dir: %v", err)
}
if err := storageworkspaces.WriteOwnershipMarker(staleTaskDir, storageworkspaces.OwnershipMarker{
TaskID: "task-stale-sym",
TaskDirName: "stale-task_sym",
LayoutVersion: storageworkspaces.LayoutVersionSemantic,
}); err != nil {
t.Fatalf("write stale ownership marker: %v", err)
}
// The symlink — the stale record's persisted path points to a symlink
// leading into the live task's checkout.
staleWorktreePath := filepath.Join(staleTaskDir, "my-repo")
if err := os.Symlink(liveWorktreePath, staleWorktreePath); err != nil {
t.Fatalf("symlink stale -> live: %v", err)
}
store := newMockStore()
store.worktrees["canonical-sym"] = &Worktree{
ID: "canonical-sym",
TaskID: "task-stale-sym",
TaskEnvironmentID: "environment-1",
RepositoryID: "repository-1",
Path: staleWorktreePath,
Branch: "feature/live",
Status: StatusActive,
}
mgr, err := NewManager(cfg, store, newTestLogger())
if err != nil {
t.Fatalf("NewManager() error = %v", err)
}
_, err = mgr.Create(context.Background(), CreateRequest{
TaskID: "task-attach-sym",
SessionID: "session-attach-sym",
TaskEnvironmentID: "environment-1",
RepositoryID: "repository-1",
RepositoryPath: repoPath,
BaseBranch: "main",
WorktreeID: "canonical-sym",
ReuseRequired: true,
})
if !errors.Is(err, ErrWorktreePathOwnedByAnotherTask) && !strings.Contains(err.Error(), "unsafe worktree path") {
t.Fatalf("Create() error = %v, want ErrWorktreePathOwnedByAnotherTask or unsafe path error", err)
}
}
// TestCreate_TryReuseExistingRejectsSymlinkedWorktreePath confirms that the
// normal reuse path (tryReuseExisting) rejects a persisted worktree whose
// path beneath tasksBase contains a symlink. Uses the session-based lookup
// branch.
func TestCreate_TryReuseExistingRejectsSymlinkedWorktreePath(t *testing.T) {
repoPath := initGitRepoWithRemote(t)
cfg := newTestConfig(t)
tasksBase := cfg.TasksBasePath
liveTaskDir := filepath.Join(tasksBase, "live-task_sym2")
if err := os.MkdirAll(liveTaskDir, 0755); err != nil {
t.Fatalf("mkdir live task dir: %v", err)
}
if err := storageworkspaces.WriteOwnershipMarker(liveTaskDir, storageworkspaces.OwnershipMarker{
TaskID: "task-live-sym2",
TaskDirName: "live-task_sym2",
LayoutVersion: storageworkspaces.LayoutVersionSemantic,
}); err != nil {
t.Fatalf("write live ownership marker: %v", err)
}
liveWorktreePath := filepath.Join(liveTaskDir, "my-repo")
runGit(t, repoPath, "worktree", "add", "-b", "feature/live2", liveWorktreePath, "main")
// Stale task: the final "repo" component is a symlink to the live checkout.
staleTaskDir := filepath.Join(tasksBase, "stale-task_sym2")
if err := os.MkdirAll(staleTaskDir, 0755); err != nil {
t.Fatalf("mkdir stale task dir: %v", err)
}
if err := storageworkspaces.WriteOwnershipMarker(staleTaskDir, storageworkspaces.OwnershipMarker{
TaskID: "task-stale-sym2",
TaskDirName: "stale-task_sym2",
LayoutVersion: storageworkspaces.LayoutVersionSemantic,
}); err != nil {
t.Fatalf("write stale ownership marker: %v", err)
}
staleWorktreePath := filepath.Join(staleTaskDir, "my-repo")
if err := os.Symlink(liveWorktreePath, staleWorktreePath); err != nil {
t.Fatalf("symlink stale2 -> live2: %v", err)
}
store := newMockStore()
store.worktrees["wt-sym2"] = &Worktree{
ID: "wt-sym2",
TaskID: "task-stale-sym2",
SessionID: "session-reuse-sym2",
RepositoryID: "repository-1",
Path: staleWorktreePath,
Branch: "feature/live2",
Status: StatusActive,
}
mgr, err := NewManager(cfg, store, newTestLogger())
if err != nil {
t.Fatalf("NewManager() error = %v", err)
}
_, err = mgr.Create(context.Background(), CreateRequest{
TaskID: "task-attach-sym2",
SessionID: "session-reuse-sym2",
RepositoryID: "repository-1",
RepositoryPath: repoPath,
BaseBranch: "main",
WorktreeID: "wt-sym2",
})
if !errors.Is(err, ErrWorktreePathOwnedByAnotherTask) && !strings.Contains(err.Error(), "unsafe worktree path") {
t.Fatalf("Create() error = %v, want ErrWorktreePathOwnedByAnotherTask or unsafe path error", err)
}
}
func TestCreate_ReuseRequiredAllowsTransferredEnvironmentOwner(t *testing.T) {
repoPath := initGitRepoWithRemote(t)
cfg := newTestConfig(t)
ownerTaskDir := filepath.Join(cfg.TasksBasePath, "owner-task_transfer")
if err := os.MkdirAll(ownerTaskDir, 0755); err != nil {
t.Fatalf("mkdir owner task dir: %v", err)
}
if err := storageworkspaces.WriteOwnershipMarker(ownerTaskDir, storageworkspaces.OwnershipMarker{
TaskID: "task-owner-transfer",
TaskDirName: "owner-task_transfer",
LayoutVersion: storageworkspaces.LayoutVersionSemantic,
}); err != nil {
t.Fatalf("write owner ownership marker: %v", err)
}
worktreePath := filepath.Join(ownerTaskDir, "repo")
runGit(t, repoPath, "worktree", "add", "-b", "feature/transferred", worktreePath, "main")
store := newMockStore()
store.worktrees["transferred-worktree"] = &Worktree{
ID: "transferred-worktree",
TaskID: "task-borrower-transfer",
TaskDirName: "owner-task_transfer",
TaskEnvironmentID: "environment-transfer",
RepositoryID: "repository-transfer",
Path: worktreePath,
Branch: "feature/transferred",
Status: StatusActive,
}
mgr, err := NewManager(cfg, store, newTestLogger())
if err != nil {
t.Fatalf("NewManager() error = %v", err)
}
got, err := mgr.Create(context.Background(), CreateRequest{
TaskID: "task-borrower-transfer",
SessionID: "session-borrower-transfer",
TaskEnvironmentID: "environment-transfer",
RepositoryID: "repository-transfer",
RepositoryPath: repoPath,
BaseBranch: "main",
WorktreeID: "transferred-worktree",
ReuseRequired: true,
})
if err != nil {
t.Fatalf("Create() error = %v, want transferred environment reuse", err)
}
if got.ID != "transferred-worktree" || got.Path != worktreePath {
t.Fatalf("Create() = %#v, want transferred worktree", got)
}
}
@@ -19,6 +19,7 @@ func (m *Manager) buildWorktreeRecord(worktreeID string, req CreateRequest, work
ID: worktreeID,
SessionID: req.SessionID,
TaskID: req.TaskID,
TaskDirName: req.TaskDirName,
TaskEnvironmentID: req.TaskEnvironmentID,
RepositoryID: req.RepositoryID,
BranchSlug: requestBranchIdentitySlug(req),
+3
View File
@@ -40,6 +40,7 @@ const worktreeSelectCols = `
ter.worktree_id,
COALESCE(s.id, '') AS session_id,
te.task_id,
COALESCE(te.task_dir_name, '') AS task_dir_name,
ter.repository_id,
r.local_path,
ter.worktree_path,
@@ -69,6 +70,7 @@ func scanWorktreeRow(row rowScanner) (*Worktree, error) {
&wt.ID,
&wt.SessionID,
&wt.TaskID,
&wt.TaskDirName,
&wt.RepositoryID,
&repositoryPath,
&wt.Path,
@@ -476,6 +478,7 @@ func (s *SQLiteStore) scanWorktrees(rows *sql.Rows) ([]*Worktree, error) {
&wt.ID,
&wt.SessionID,
&wt.TaskID,
&wt.TaskDirName,
&wt.RepositoryID,
&repositoryPath,
&wt.Path,
@@ -84,6 +84,44 @@ func TestSQLiteStore_ReinitializesSchema(t *testing.T) {
}
}
func TestSQLiteStore_ProjectsStableTaskDirName(t *testing.T) {
store := newTestStore(t)
ctx := context.Background()
store.seedSessionWithEnvironment(t, "session-stable-root", "task-current-owner")
if _, err := store.db.ExecContext(ctx, `
UPDATE task_environments
SET task_dir_name = ?
WHERE id = ?
`, "original-task-root_abc", "env-session-stable-root"); err != nil {
t.Fatalf("set task directory name: %v", err)
}
wt := &Worktree{
ID: "wt-stable-root",
SessionID: "session-stable-root",
RepositoryID: "repo-stable-root",
Path: "/tmp/stable-root/repo",
Branch: "feature/stable-root",
Status: StatusActive,
CreatedAt: time.Now(),
UpdatedAt: time.Now(),
}
if err := store.CreateWorktree(ctx, wt); err != nil {
t.Fatalf("create worktree: %v", err)
}
got, err := store.GetWorktreeByID(ctx, wt.ID)
if err != nil {
t.Fatalf("get worktree: %v", err)
}
if got == nil {
t.Fatal("get worktree returned nil")
}
if got.TaskDirName != "original-task-root_abc" {
t.Fatalf("TaskDirName = %q, want original-task-root_abc", got.TaskDirName)
}
}
func TestSQLiteStore_ListActiveWorktreePaths(t *testing.T) {
store := newTestStore(t)
ctx := context.Background()
@@ -40,6 +40,11 @@ type Worktree struct {
// Multiple worktrees can exist for the same task (one per agent session).
TaskID string `json:"task_id"`
// TaskDirName is the stable task-root identity used by filesystem
// ownership checks. It remains unchanged when a task environment changes
// owners, so it is intentionally not part of the public JSON contract.
TaskDirName string `json:"-"`
// TaskEnvironmentID is the task environment that owns this worktree.
// Physical worktree records live on task_environment_repos; sessions
// reference the environment through task_sessions.task_environment_id.