fix(worktree): stop planting self-referential links in local repositories (#2007)
This commit is contained in:
@@ -472,7 +472,7 @@ func (m *Manager) createExecution(ctx context.Context, taskID string, info *Work
|
||||
return nil, err
|
||||
}
|
||||
if info.ExecutorType == string(models.ExecutorTypeLocal) || info.ExecutorType == "local_pc" {
|
||||
if err := reconcileWorkspaceRepositories(info.WorkspacePath, info.WorkspaceRepositories); err != nil {
|
||||
if err := reconcileWorkspaceRepositories(info.WorkspacePath, info.WorkspaceRepositories, m.logger); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1068,7 +1068,7 @@ func (m *Manager) launchInternal(ctx context.Context, req *LaunchRequest) (*Agen
|
||||
return nil, err
|
||||
}
|
||||
if req.ExecutorType == string(models.ExecutorTypeLocal) || req.ExecutorType == legacyExecutorTypeLocalPC {
|
||||
if err := reconcileWorkspaceRepositories(workspacePath, workspaceRepositorySpecsFromLaunch(req)); err != nil {
|
||||
if err := reconcileWorkspaceRepositories(workspacePath, workspaceRepositorySpecsFromLaunch(req), m.logger); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
}
|
||||
|
||||
@@ -6,6 +6,9 @@ import (
|
||||
"os"
|
||||
"path/filepath"
|
||||
|
||||
"go.uber.org/zap"
|
||||
|
||||
"github.com/kandev/kandev/internal/common/logger"
|
||||
"github.com/kandev/kandev/internal/worktree"
|
||||
)
|
||||
|
||||
@@ -19,7 +22,7 @@ func reconcileWorkspaceSources(_ context.Context, root string, folders []Workspa
|
||||
return fmt.Errorf("workspace root is required for durable folders")
|
||||
}
|
||||
for _, folder := range folders {
|
||||
if folder.Name == "" || filepath.Base(folder.Name) != folder.Name || folder.LocalPath == "" {
|
||||
if !isWorkspaceEntryName(folder.Name) || folder.LocalPath == "" {
|
||||
return fmt.Errorf("invalid durable workspace folder")
|
||||
}
|
||||
info, err := os.Stat(folder.LocalPath)
|
||||
@@ -33,7 +36,16 @@ func reconcileWorkspaceSources(_ context.Context, root string, folders []Workspa
|
||||
return nil
|
||||
}
|
||||
|
||||
func reconcileWorkspaceRepositories(root string, repositories []WorkspaceRepositorySpec) error {
|
||||
// reconcileWorkspaceRepositories recreates Kandev-owned repository links below
|
||||
// root. A spec whose repository IS the workspace root is skipped: for the local
|
||||
// executor the primary repository is the workspace root itself, so linking it
|
||||
// would plant a self-referential junction/symlink inside the user's own
|
||||
// checkout. This mirrors buildRemoteWorkspaceRepositories, which skips the
|
||||
// primary for the same reason. The comparison is by filesystem identity, not
|
||||
// by index, because a host-materialized multi-repo local task roots the
|
||||
// workspace at ~/.kandev/tasks/<taskDir> — there repositories[0] is a real
|
||||
// sibling and must keep its link.
|
||||
func reconcileWorkspaceRepositories(root string, repositories []WorkspaceRepositorySpec, log *logger.Logger) error {
|
||||
if len(repositories) == 0 {
|
||||
return nil
|
||||
}
|
||||
@@ -41,9 +53,13 @@ func reconcileWorkspaceRepositories(root string, repositories []WorkspaceReposit
|
||||
return fmt.Errorf("workspace root is required for durable repositories")
|
||||
}
|
||||
for _, repository := range repositories {
|
||||
if repository.RepoName == "" || filepath.Base(repository.RepoName) != repository.RepoName || repository.RepositoryPath == "" {
|
||||
if !isWorkspaceEntryName(repository.RepoName) || repository.RepositoryPath == "" {
|
||||
return fmt.Errorf("invalid durable workspace repository")
|
||||
}
|
||||
if sameDirectory(root, repository.RepositoryPath) {
|
||||
warnSelfReferentialEntry(root, repository.RepoName, log)
|
||||
continue
|
||||
}
|
||||
info, err := os.Stat(repository.RepositoryPath)
|
||||
if err != nil || !info.IsDir() {
|
||||
return fmt.Errorf("workspace repository %q target is missing: %s", repository.RepoName, repository.RepositoryPath)
|
||||
@@ -55,6 +71,80 @@ func reconcileWorkspaceRepositories(root string, repositories []WorkspaceReposit
|
||||
return nil
|
||||
}
|
||||
|
||||
// selfReferentialEntryWarning is shared with the tests that assert the user is
|
||||
// told about a stale entry, so the two cannot drift apart silently.
|
||||
const selfReferentialEntryWarning = "workspace entry links to the workspace root; remove that entry to stop tools recursing into it"
|
||||
|
||||
// warnSelfReferentialEntry surfaces a link an earlier release planted inside
|
||||
// the user's own repository, and deliberately leaves it in place.
|
||||
//
|
||||
// Kandev writes no ownership marker into user-owned sources, so such an entry
|
||||
// cannot be shown to be ours: a user, or the repository itself, may keep a link
|
||||
// of the same name and target on purpose, and deleting it would destroy content
|
||||
// that is not ours. A stat-then-remove sequence could not close the window
|
||||
// between the check and the unlink either — the entry can be swapped for an
|
||||
// unrelated empty directory in between, which os.Remove would happily delete.
|
||||
// Reporting it costs the user one command and risks nothing.
|
||||
func warnSelfReferentialEntry(root, name string, log *logger.Logger) {
|
||||
if log == nil {
|
||||
return
|
||||
}
|
||||
selfLink, err := worktree.IsSelfReferentialDirectoryLink(root, name)
|
||||
if err != nil || !selfLink {
|
||||
return
|
||||
}
|
||||
// The guidance is deliberately shell-neutral. A ready-to-paste command built
|
||||
// from the entry path would need per-shell escaping, and a path carrying
|
||||
// $(), backticks or %VAR% would otherwise reach a shell that expands it.
|
||||
log.Warn(selfReferentialEntryWarning,
|
||||
zap.String("entry", filepath.Join(root, name)),
|
||||
zap.String("note", "the entry is a directory link, not a copy: removing it leaves the repository untouched"))
|
||||
}
|
||||
|
||||
// isWorkspaceEntryName reports whether name is usable as a single entry below
|
||||
// an owned workspace root. "." and ".." survive a filepath.Base round-trip, so
|
||||
// they are rejected explicitly here rather than deeper in the worktree helpers,
|
||||
// which would surface them as a confusing "owned link entry already exists".
|
||||
func isWorkspaceEntryName(name string) bool {
|
||||
if name == "" || name == "." || name == ".." {
|
||||
return false
|
||||
}
|
||||
if filepath.Base(name) != name {
|
||||
return false
|
||||
}
|
||||
// A bare root survives the Base round-trip too — filepath.Base("/") is "/"
|
||||
// on Unix and filepath.Base(`\`) is `\` on Windows — and joining it resolves
|
||||
// back to the root itself rather than to an entry below it.
|
||||
return name != "/" && name != string(filepath.Separator) && filepath.VolumeName(name) == ""
|
||||
}
|
||||
|
||||
// sameDirectory reports whether two paths name the same directory on disk.
|
||||
// The comparison is filesystem identity rather than path text: os.Stat follows
|
||||
// junctions and Unix symlinks alike, and os.SameFile compares volume and file
|
||||
// index, which also absorbs 8.3 short paths and path case. A canonical-path
|
||||
// comparison would not do — filepath.EvalSymlinks does not traverse a Windows
|
||||
// junction, it returns the link's own path.
|
||||
//
|
||||
// A path that cannot be stat'ed is not the same directory: the workspace root
|
||||
// may not exist yet, and the caller must then fall through to link creation.
|
||||
// Both sides must be directories, so that a repository path replaced by a
|
||||
// regular file falls through to the caller's IsDir validation instead of being
|
||||
// skipped as "already the root".
|
||||
func sameDirectory(left, right string) bool {
|
||||
if left == "" || right == "" {
|
||||
return false
|
||||
}
|
||||
leftInfo, err := os.Stat(left)
|
||||
if err != nil || !leftInfo.IsDir() {
|
||||
return false
|
||||
}
|
||||
rightInfo, err := os.Stat(right)
|
||||
if err != nil || !rightInfo.IsDir() {
|
||||
return false
|
||||
}
|
||||
return os.SameFile(leftInfo, rightInfo)
|
||||
}
|
||||
|
||||
func workspaceRepositorySpecsFromLaunch(req *LaunchRequest) []WorkspaceRepositorySpec {
|
||||
if req == nil {
|
||||
return nil
|
||||
|
||||
@@ -4,7 +4,15 @@ import (
|
||||
"context"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"runtime"
|
||||
"testing"
|
||||
|
||||
"go.uber.org/zap"
|
||||
"go.uber.org/zap/zapcore"
|
||||
"go.uber.org/zap/zaptest/observer"
|
||||
|
||||
"github.com/kandev/kandev/internal/common/logger"
|
||||
"github.com/kandev/kandev/internal/worktree"
|
||||
)
|
||||
|
||||
func TestReconcileWorkspaceSources_RejectsMissingFolderTarget(t *testing.T) {
|
||||
@@ -16,16 +24,221 @@ func TestReconcileWorkspaceSources_RejectsMissingFolderTarget(t *testing.T) {
|
||||
|
||||
func TestReconcileWorkspaceRepositories_RecreatesMissingOwnedLink(t *testing.T) {
|
||||
root, source := t.TempDir(), t.TempDir()
|
||||
if err := reconcileWorkspaceRepositories(root, []WorkspaceRepositorySpec{{RepoName: "api", RepositoryPath: source}}); err != nil {
|
||||
writeMarker(t, source)
|
||||
if err := reconcileWorkspaceRepositories(root, []WorkspaceRepositorySpec{{RepoName: "api", RepositoryPath: source}}, nil); err != nil {
|
||||
t.Fatalf("reconcileWorkspaceRepositories: %v", err)
|
||||
}
|
||||
if got, err := os.Readlink(filepath.Join(root, "api")); err != nil || got != source {
|
||||
t.Fatalf("repository link = %q, %v; want %q", got, err, source)
|
||||
// Read through the link instead of comparing os.Readlink output: Go
|
||||
// normalizes a junction target, so it never string-equals a t.TempDir()
|
||||
// carrying an 8.3 component such as C:\Users\JOHNDO~1.
|
||||
if got, err := os.ReadFile(filepath.Join(root, "api", "live.txt")); err != nil || string(got) != "one" {
|
||||
t.Fatalf("read through repository link = %q, %v", got, err)
|
||||
}
|
||||
if err := os.Remove(filepath.Join(root, "api")); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := reconcileWorkspaceRepositories(root, []WorkspaceRepositorySpec{{RepoName: "api", RepositoryPath: source}}); err != nil {
|
||||
if err := reconcileWorkspaceRepositories(root, []WorkspaceRepositorySpec{{RepoName: "api", RepositoryPath: source}}, nil); err != nil {
|
||||
t.Fatalf("reconcile after reset: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
// writeMarker seeds a file used to prove a directory survived reconciliation,
|
||||
// or that a link resolves to it.
|
||||
func writeMarker(t *testing.T, dir string) string {
|
||||
t.Helper()
|
||||
path := filepath.Join(dir, "live.txt")
|
||||
if err := os.WriteFile(path, []byte("one"), 0o644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
return path
|
||||
}
|
||||
|
||||
// The local executor roots the workspace at the primary repository itself, so
|
||||
// linking that repository would plant a self-referential junction inside the
|
||||
// user's own checkout.
|
||||
func TestReconcileWorkspaceRepositories_SkipsRepositoryThatIsWorkspaceRoot(t *testing.T) {
|
||||
root := t.TempDir()
|
||||
marker := writeMarker(t, root)
|
||||
|
||||
if err := reconcileWorkspaceRepositories(root, []WorkspaceRepositorySpec{{RepoName: "api", RepositoryPath: root}}, nil); err != nil {
|
||||
t.Fatalf("reconcileWorkspaceRepositories: %v", err)
|
||||
}
|
||||
if _, err := os.Lstat(filepath.Join(root, "api")); !os.IsNotExist(err) {
|
||||
t.Fatalf("self-referential entry created inside the repository: %v", err)
|
||||
}
|
||||
if got, err := os.ReadFile(marker); err != nil || string(got) != "one" {
|
||||
t.Fatalf("workspace root content = %q, %v", got, err)
|
||||
}
|
||||
}
|
||||
|
||||
// A self-link planted by an earlier release is reported, never deleted: it
|
||||
// cannot be shown to be Kandev-owned, so removing it could destroy a link the
|
||||
// user or the repository keeps on purpose. Preserving it is only half the
|
||||
// contract though — the user has to learn the entry is there — so the warning
|
||||
// and its structured entry path are asserted too.
|
||||
func TestReconcileWorkspaceRepositories_PreservesAndReportsPreExistingSelfLink(t *testing.T) {
|
||||
root := t.TempDir()
|
||||
marker := writeMarker(t, root)
|
||||
if _, err := worktree.CreateOwnedDirectoryLink(root, "api", root); err != nil {
|
||||
t.Fatalf("seed self link: %v", err)
|
||||
}
|
||||
core, logs := observer.New(zapcore.WarnLevel)
|
||||
log, err := logger.NewFromZap(zap.New(core))
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
if err := reconcileWorkspaceRepositories(root, []WorkspaceRepositorySpec{{RepoName: "api", RepositoryPath: root}}, log); err != nil {
|
||||
t.Fatalf("reconcileWorkspaceRepositories: %v", err)
|
||||
}
|
||||
if _, err := os.Lstat(filepath.Join(root, "api")); err != nil {
|
||||
t.Fatalf("pre-existing self link was removed: %v", err)
|
||||
}
|
||||
if got, err := os.ReadFile(marker); err != nil || string(got) != "one" {
|
||||
t.Fatalf("workspace root content = %q, %v", got, err)
|
||||
}
|
||||
|
||||
entries := logs.FilterMessage(selfReferentialEntryWarning).All()
|
||||
if len(entries) != 1 {
|
||||
t.Fatalf("warnings = %d, want 1", len(entries))
|
||||
}
|
||||
if got, want := entries[0].ContextMap()["entry"], filepath.Join(root, "api"); got != want {
|
||||
t.Fatalf("entry = %v, want %v", got, want)
|
||||
}
|
||||
}
|
||||
|
||||
// A repository that is the workspace root but carries no stale entry must stay
|
||||
// silent: there is nothing for the user to clean up.
|
||||
func TestReconcileWorkspaceRepositories_DoesNotWarnWithoutSelfLink(t *testing.T) {
|
||||
root := t.TempDir()
|
||||
core, logs := observer.New(zapcore.WarnLevel)
|
||||
log, err := logger.NewFromZap(zap.New(core))
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
if err := reconcileWorkspaceRepositories(root, []WorkspaceRepositorySpec{{RepoName: "api", RepositoryPath: root}}, log); err != nil {
|
||||
t.Fatalf("reconcileWorkspaceRepositories: %v", err)
|
||||
}
|
||||
if got := logs.FilterMessage(selfReferentialEntryWarning).Len(); got != 0 {
|
||||
t.Fatalf("warnings = %d, want 0", got)
|
||||
}
|
||||
}
|
||||
|
||||
// The guard is per spec, not a blanket skip: siblings still need their links.
|
||||
func TestReconcileWorkspaceRepositories_LinksSiblingWhenPrimaryIsWorkspaceRoot(t *testing.T) {
|
||||
root, sibling := t.TempDir(), t.TempDir()
|
||||
writeMarker(t, sibling)
|
||||
|
||||
err := reconcileWorkspaceRepositories(root, []WorkspaceRepositorySpec{
|
||||
{RepoName: "api", RepositoryPath: root},
|
||||
{RepoName: "libs", RepositoryPath: sibling},
|
||||
}, nil)
|
||||
if err != nil {
|
||||
t.Fatalf("reconcileWorkspaceRepositories: %v", err)
|
||||
}
|
||||
if _, err := os.Lstat(filepath.Join(root, "api")); !os.IsNotExist(err) {
|
||||
t.Fatalf("self-referential entry created for the primary: %v", err)
|
||||
}
|
||||
if got, err := os.ReadFile(filepath.Join(root, "libs", "live.txt")); err != nil || string(got) != "one" {
|
||||
t.Fatalf("sibling link = %q, %v; siblings must still be linked", got, err)
|
||||
}
|
||||
}
|
||||
|
||||
// Drives the exact production wiring of manager_launch.go: a single-repo local
|
||||
// LaunchRequest, whose specs are synthesized by RepoSpecs() rather than built
|
||||
// by hand, reconciled against the workspace path that launchResolveWorkspacePath
|
||||
// derives for a non-worktree executor — the repository itself.
|
||||
func TestReconcileWorkspaceRepositories_LocalLaunchRequestPlantsNoSelfLink(t *testing.T) {
|
||||
repo := t.TempDir()
|
||||
marker := writeMarker(t, repo)
|
||||
req := &LaunchRequest{
|
||||
ExecutorType: "local",
|
||||
RepositoryID: "repo-1",
|
||||
RepositoryPath: repo,
|
||||
RepoName: filepath.Base(repo),
|
||||
}
|
||||
|
||||
// launchResolveWorkspacePath returns req.RepositoryPath when WorkspacePath
|
||||
// is empty and the executor is not worktree-backed.
|
||||
if err := reconcileWorkspaceRepositories(repo, workspaceRepositorySpecsFromLaunch(req), nil); err != nil {
|
||||
t.Fatalf("reconcileWorkspaceRepositories: %v", err)
|
||||
}
|
||||
if _, err := os.Lstat(filepath.Join(repo, req.RepoName)); !os.IsNotExist(err) {
|
||||
t.Fatalf("local launch planted a self-referential entry: %v", err)
|
||||
}
|
||||
if got, err := os.ReadFile(marker); err != nil || string(got) != "one" {
|
||||
t.Fatalf("repository content = %q, %v", got, err)
|
||||
}
|
||||
}
|
||||
|
||||
// A repository path replaced by a regular file must surface as a missing
|
||||
// target. Comparing by identity alone would report the file as "already the
|
||||
// workspace root" and skip the IsDir validation, letting the launch continue
|
||||
// with a workspace path that is not a directory.
|
||||
func TestReconcileWorkspaceRepositories_RejectsFileAsRepositoryPath(t *testing.T) {
|
||||
file := filepath.Join(t.TempDir(), "not-a-directory")
|
||||
if err := os.WriteFile(file, []byte("one"), 0o644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
err := reconcileWorkspaceRepositories(file, []WorkspaceRepositorySpec{{RepoName: "api", RepositoryPath: file}}, nil)
|
||||
if err == nil {
|
||||
t.Fatal("a regular file was accepted as both workspace root and repository")
|
||||
}
|
||||
}
|
||||
|
||||
func TestIsWorkspaceEntryName(t *testing.T) {
|
||||
accepted := []string{"api", "libs", "my-service", "repo.git"}
|
||||
rejected := []string{"", ".", "..", "/", "a/b", string(filepath.Separator)}
|
||||
|
||||
// A backslash separates path elements only on Windows, and only there does a
|
||||
// drive letter prefix a path. On Unix all three are ordinary characters in a
|
||||
// legal single-component name, so the expectation flips with the platform.
|
||||
if runtime.GOOS == "windows" {
|
||||
rejected = append(rejected, `\`, "C:", `a\b`)
|
||||
} else {
|
||||
accepted = append(accepted, `\`, "C:", `a\b`)
|
||||
}
|
||||
|
||||
for _, name := range accepted {
|
||||
if !isWorkspaceEntryName(name) {
|
||||
t.Errorf("isWorkspaceEntryName(%q) = false, want true", name)
|
||||
}
|
||||
}
|
||||
for _, name := range rejected {
|
||||
if isWorkspaceEntryName(name) {
|
||||
t.Errorf("isWorkspaceEntryName(%q) = true, want false", name)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// ".", ".." and a bare filesystem root all survive a filepath.Base round-trip,
|
||||
// so they need rejecting here rather than deeper in the owned-link helpers:
|
||||
// joining any of them resolves back to the root itself instead of to an entry
|
||||
// below it. Only names invalid on every platform belong here — the
|
||||
// Windows-specific ones are covered by TestIsWorkspaceEntryName.
|
||||
func TestReconcileWorkspaceRepositories_RejectsTraversalRepoName(t *testing.T) {
|
||||
root, source := t.TempDir(), t.TempDir()
|
||||
for _, name := range []string{".", "..", "/", string(filepath.Separator)} {
|
||||
if err := reconcileWorkspaceRepositories(root, []WorkspaceRepositorySpec{{RepoName: name, RepositoryPath: source}}, nil); err == nil {
|
||||
t.Fatalf("RepoName %q was accepted", name)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// A host-materialized task roots the workspace at ~/.kandev/tasks/<taskDir>,
|
||||
// where the first repository is a real sibling — so the guard must not degrade
|
||||
// into skipping index 0.
|
||||
func TestReconcileWorkspaceRepositories_LinksPrimaryWhenRootIsTaskDirectory(t *testing.T) {
|
||||
root := filepath.Join(t.TempDir(), "tasks", "task-1")
|
||||
primary := t.TempDir()
|
||||
writeMarker(t, primary)
|
||||
|
||||
if err := reconcileWorkspaceRepositories(root, []WorkspaceRepositorySpec{{RepoName: "api", RepositoryPath: primary}}, nil); err != nil {
|
||||
t.Fatalf("reconcileWorkspaceRepositories: %v", err)
|
||||
}
|
||||
if got, err := os.ReadFile(filepath.Join(root, "api", "live.txt")); err != nil || string(got) != "one" {
|
||||
t.Fatalf("primary link = %q, %v; the primary must be linked into a Kandev task root", got, err)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -79,8 +79,16 @@ func verifyCreatedOwnedDirectoryLink(root, link string) error {
|
||||
}
|
||||
|
||||
// EnsureOwnedDirectoryLink returns an existing matching live link or creates
|
||||
// it. A non-link/collision, or a link to another canonical directory, fails
|
||||
// closed and is never replaced.
|
||||
// it. A non-link/collision, or a link to another directory, fails closed and is
|
||||
// never replaced.
|
||||
//
|
||||
// The existing link is matched by filesystem identity, not by resolved path.
|
||||
// filepath.EvalSymlinks does not traverse a Windows junction — it returns the
|
||||
// link's own normalized path — so a path comparison rejected every unchanged
|
||||
// junction as a target mismatch, and each relaunch or resume of a task with
|
||||
// workspace source links failed. os.Stat follows a junction and a Unix symlink
|
||||
// alike, and os.SameFile compares volume and file index, which is also immune
|
||||
// to 8.3 short paths and path case.
|
||||
func EnsureOwnedDirectoryLink(root, name, target string) (string, bool, error) {
|
||||
link := filepath.Join(root, name)
|
||||
info, err := os.Lstat(link)
|
||||
@@ -88,15 +96,15 @@ func EnsureOwnedDirectoryLink(root, name, target string) (string, bool, error) {
|
||||
if !isPlatformDirectoryLink(info, link) {
|
||||
return "", false, fmt.Errorf("owned link entry already exists: %s", name)
|
||||
}
|
||||
actual, err := filepath.EvalSymlinks(link)
|
||||
actual, err := os.Stat(link)
|
||||
if err != nil {
|
||||
return "", false, fmt.Errorf("resolve owned link: %w", err)
|
||||
}
|
||||
expected, err := filepath.EvalSymlinks(target)
|
||||
expected, err := os.Stat(target)
|
||||
if err != nil {
|
||||
return "", false, fmt.Errorf("canonicalize link target: %w", err)
|
||||
return "", false, fmt.Errorf("inspect link target: %w", err)
|
||||
}
|
||||
if filepath.Clean(actual) != filepath.Clean(expected) {
|
||||
if !os.SameFile(actual, expected) {
|
||||
return "", false, fmt.Errorf("owned link target mismatch: %s", name)
|
||||
}
|
||||
return link, false, nil
|
||||
@@ -108,6 +116,55 @@ func EnsureOwnedDirectoryLink(root, name, target string) (string, bool, error) {
|
||||
return created, err == nil, err
|
||||
}
|
||||
|
||||
// IsSelfReferentialDirectoryLink reports whether root/name is a platform
|
||||
// directory link whose target is root itself — the shape an earlier release
|
||||
// planted inside a user's own repository for local-executor tasks, and which
|
||||
// EnsureOwnedDirectoryLink then accepts as valid forever.
|
||||
//
|
||||
// It deliberately only reports. Such an entry cannot be shown to be
|
||||
// Kandev-owned: Kandev writes no ownership marker into user-owned sources, and
|
||||
// a user (or the repository itself) may keep a link of the same name and target
|
||||
// on purpose, so removing it could destroy content that is not ours. Nor could
|
||||
// a stat-then-remove sequence close the window between the check and the
|
||||
// unlink. Callers surface it and leave removal to the user.
|
||||
func IsSelfReferentialDirectoryLink(root, name string) (bool, error) {
|
||||
if !isOwnedDirectoryLinkPath(root, name) {
|
||||
return false, fmt.Errorf("invalid owned link path")
|
||||
}
|
||||
link, err := ownedDirectoryLinkPath(root, name)
|
||||
if err != nil {
|
||||
return false, err
|
||||
}
|
||||
info, err := os.Lstat(link)
|
||||
if os.IsNotExist(err) {
|
||||
return false, nil
|
||||
}
|
||||
if err != nil {
|
||||
return false, fmt.Errorf("inspect owned link entry: %w", err)
|
||||
}
|
||||
if !isPlatformDirectoryLink(info, link) {
|
||||
return false, nil
|
||||
}
|
||||
return linkTargetsRoot(root, link)
|
||||
}
|
||||
|
||||
// linkTargetsRoot compares filesystem identity, not resolved path text.
|
||||
// filepath.EvalSymlinks does not traverse a Windows junction — it returns the
|
||||
// link's own normalized path — so a path comparison never matches. os.Stat
|
||||
// follows a junction and a Unix symlink alike, and os.SameFile then compares
|
||||
// volume and file index, which also absorbs 8.3 short paths and case.
|
||||
func linkTargetsRoot(root, link string) (bool, error) {
|
||||
linkInfo, err := os.Stat(link)
|
||||
if err != nil {
|
||||
return false, fmt.Errorf("resolve owned link: %w", err)
|
||||
}
|
||||
rootInfo, err := os.Stat(root)
|
||||
if err != nil {
|
||||
return false, fmt.Errorf("resolve owned root: %w", err)
|
||||
}
|
||||
return os.SameFile(linkInfo, rootInfo), nil
|
||||
}
|
||||
|
||||
func mkdirOwned(root string) error {
|
||||
parent := filepath.Dir(root)
|
||||
if err := requireNoSymlinkAncestors(parent); err != nil {
|
||||
|
||||
@@ -55,3 +55,114 @@ func TestCreateOwnedDirectoryLinkRejectsSymlinkedControlAncestor(t *testing.T) {
|
||||
t.Fatal("CreateOwnedDirectoryLink accepted symlinked control ancestor")
|
||||
}
|
||||
}
|
||||
|
||||
// seedOwnedDirectoryLink plants a link through the production creation path so
|
||||
// the test exercises the same reparse point / symlink a real launch produced.
|
||||
// Creation failure is fatal, not a skip: a directory junction needs no elevation
|
||||
// on Windows and a symlink is always available on the Unix runners, so a failure
|
||||
// here is a real regression rather than an unsupported platform.
|
||||
func seedOwnedDirectoryLink(t *testing.T, root, name, target string) {
|
||||
t.Helper()
|
||||
if _, err := CreateOwnedDirectoryLink(root, name, target); err != nil {
|
||||
t.Fatalf("CreateOwnedDirectoryLink: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
// The predicate only reports. Every case below additionally asserts that the
|
||||
// inspected entry is still on disk afterwards, since it may be a link the user
|
||||
// or the repository keeps on purpose.
|
||||
func TestIsSelfReferentialDirectoryLinkDetectsSelfLink(t *testing.T) {
|
||||
root := t.TempDir()
|
||||
seedOwnedDirectoryLink(t, root, "self", root)
|
||||
|
||||
selfLink, err := IsSelfReferentialDirectoryLink(root, "self")
|
||||
if err != nil || !selfLink {
|
||||
t.Fatalf("IsSelfReferentialDirectoryLink = %v, %v; want true, nil", selfLink, err)
|
||||
}
|
||||
if _, err := os.Lstat(filepath.Join(root, "self")); err != nil {
|
||||
t.Fatalf("entry was removed: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestIsSelfReferentialDirectoryLinkIgnoresForeignLink(t *testing.T) {
|
||||
root, target := t.TempDir(), t.TempDir()
|
||||
if err := os.WriteFile(filepath.Join(target, "live.txt"), []byte("one"), 0o644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
seedOwnedDirectoryLink(t, root, "source", target)
|
||||
|
||||
selfLink, err := IsSelfReferentialDirectoryLink(root, "source")
|
||||
if err != nil || selfLink {
|
||||
t.Fatalf("IsSelfReferentialDirectoryLink = %v, %v; want false, nil", selfLink, err)
|
||||
}
|
||||
if got, err := os.ReadFile(filepath.Join(root, "source", "live.txt")); err != nil || string(got) != "one" {
|
||||
t.Fatalf("read through kept link = %q, %v", got, err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestIsSelfReferentialDirectoryLinkIgnoresRealDirectory(t *testing.T) {
|
||||
root := t.TempDir()
|
||||
real := filepath.Join(root, "api")
|
||||
if err := os.Mkdir(real, 0o755); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := os.WriteFile(filepath.Join(real, "live.txt"), []byte("one"), 0o644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
selfLink, err := IsSelfReferentialDirectoryLink(root, "api")
|
||||
if err != nil || selfLink {
|
||||
t.Fatalf("IsSelfReferentialDirectoryLink = %v, %v; want false, nil", selfLink, err)
|
||||
}
|
||||
if got, err := os.ReadFile(filepath.Join(real, "live.txt")); err != nil || string(got) != "one" {
|
||||
t.Fatalf("real directory content = %q, %v", got, err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestIsSelfReferentialDirectoryLinkIgnoresMissingEntry(t *testing.T) {
|
||||
selfLink, err := IsSelfReferentialDirectoryLink(t.TempDir(), "absent")
|
||||
if err != nil || selfLink {
|
||||
t.Fatalf("IsSelfReferentialDirectoryLink = %v, %v; want false, nil", selfLink, err)
|
||||
}
|
||||
}
|
||||
|
||||
// Every relaunch and resume re-runs Ensure over unchanged links, so a second
|
||||
// call must be a no-op. A path-text comparison rejected Windows junctions here
|
||||
// because filepath.EvalSymlinks does not traverse them.
|
||||
func TestEnsureOwnedDirectoryLinkIsIdempotent(t *testing.T) {
|
||||
root := filepath.Join(t.TempDir(), "tasks", "task-1")
|
||||
target := t.TempDir()
|
||||
if err := os.WriteFile(filepath.Join(target, "live.txt"), []byte("one"), 0o644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
first, created, err := EnsureOwnedDirectoryLink(root, "api", target)
|
||||
if err != nil || !created {
|
||||
t.Fatalf("first EnsureOwnedDirectoryLink: created=%v err=%v", created, err)
|
||||
}
|
||||
|
||||
second, created, err := EnsureOwnedDirectoryLink(root, "api", target)
|
||||
if err != nil {
|
||||
t.Fatalf("second EnsureOwnedDirectoryLink: %v; an unchanged link must be reused", err)
|
||||
}
|
||||
if created {
|
||||
t.Fatal("second EnsureOwnedDirectoryLink reported creation")
|
||||
}
|
||||
if second != first {
|
||||
t.Fatalf("second link = %q, want %q", second, first)
|
||||
}
|
||||
if got, err := os.ReadFile(filepath.Join(second, "live.txt")); err != nil || string(got) != "one" {
|
||||
t.Fatalf("read through reused link = %q, %v", got, err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestEnsureOwnedDirectoryLinkRejectsDifferentTarget(t *testing.T) {
|
||||
root := filepath.Join(t.TempDir(), "tasks", "task-1")
|
||||
target, other := t.TempDir(), t.TempDir()
|
||||
if _, _, err := EnsureOwnedDirectoryLink(root, "api", target); err != nil {
|
||||
t.Fatalf("EnsureOwnedDirectoryLink: %v", err)
|
||||
}
|
||||
|
||||
if _, _, err := EnsureOwnedDirectoryLink(root, "api", other); err == nil {
|
||||
t.Fatal("EnsureOwnedDirectoryLink accepted a link pointing elsewhere")
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user