The mkdir->rename->remove->rename directory fallback had a TOCTOU window between the second remove and the second rename: a concurrent writer creating an entry at the destination was silently clobbered. The fallback now claims the destination once with mkdir and never unlinks it: the fast-path rename publishes over the claim (Linux), and platforms that refuse directory renames (macOS, Windows) move the staged children into the claim through atomic no-clobber primitives (mkdir/os.Link/os.Symlink), consuming the emptied source shell on success. renameSkillPathNoReplace now returns the mkdir-claim identity captured by the child-move path. PublishSkillPathNoReplace uses it as a three-state ownership witness: the atomic/fast paths keep the staged-inode proof, the child-move path proves dest is still the mkdir claim, and a mismatch reports the new ErrSkillPathPublicationUncertain sentinel with the destination retained. The witness is real on POSIX now: darwin and linux report the dev:ino file identity instead of the empty no-op. Upstream consumers honor the sentinel: the mono/multi upgrade copy fallbacks no longer retry over an uncertain destination (the retry would displace the concurrent writer's object), and skill setup reports the retained destination instead of claiming a rollback. Rewrites the fallback tests that pinned the removed remove-and-retry flow and adds regression coverage: concurrent claim entries abort with the destination retained, wholesale replacement after child-move reports the uncertain sentinel, staged-set transactions pass the sentinel through, and both copy fallbacks short-circuit (ablation-verified).
452 lines
15 KiB
Go
452 lines
15 KiB
Go
package upgrade
|
|
|
|
import (
|
|
"errors"
|
|
"os"
|
|
"path/filepath"
|
|
"runtime"
|
|
"strings"
|
|
"testing"
|
|
|
|
"github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/testseam"
|
|
)
|
|
|
|
func TestCrossPlatformCoverageSkillPathCrossDeviceMove(t *testing.T) {
|
|
t.Run("directory file and lexical links", func(t *testing.T) {
|
|
root := t.TempDir()
|
|
t.Cleanup(func() { makeSkillPathTreeWritable(root) })
|
|
src := filepath.Join(root, "external", "dingtalk-chat")
|
|
dst := filepath.Join(root, "home", ".dws", "skill-backups", "stamp", "dingtalk-chat")
|
|
if err := os.MkdirAll(filepath.Join(src, "references"), 0o750); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
if err := os.WriteFile(filepath.Join(src, "SKILL.md"), []byte("chat\n"), 0o640); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
if err := os.WriteFile(filepath.Join(src, "references", "guide.md"), []byte("guide\n"), 0o600); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
linksCreated := true
|
|
if err := os.Symlink("SKILL.md", filepath.Join(src, "skill-link")); err != nil {
|
|
if runtime.GOOS != "windows" {
|
|
t.Fatal(err)
|
|
}
|
|
linksCreated = false
|
|
}
|
|
if linksCreated {
|
|
if err := os.Symlink("missing.md", filepath.Join(src, "dangling-link")); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
}
|
|
|
|
forceCrossDeviceRename(t, src, dst)
|
|
if err := moveSkillPathRecoverably(src, dst); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
if _, err := os.Lstat(src); !os.IsNotExist(err) {
|
|
t.Fatalf("source must be removed: %v", err)
|
|
}
|
|
if got, err := os.ReadFile(filepath.Join(dst, "SKILL.md")); err != nil || string(got) != "chat\n" {
|
|
t.Fatalf("copied file = %q, %v", got, err)
|
|
}
|
|
if info, err := os.Stat(filepath.Join(dst, "SKILL.md")); err != nil {
|
|
t.Fatalf("copied file stat = %v, %v", info, err)
|
|
} else if runtime.GOOS != "windows" && info.Mode().Perm() != 0o640 {
|
|
t.Fatalf("copied file mode = %v; want 0640", info.Mode().Perm())
|
|
}
|
|
if linksCreated {
|
|
for name, want := range map[string]string{"skill-link": "SKILL.md", "dangling-link": "missing.md"} {
|
|
info, err := os.Lstat(filepath.Join(dst, name))
|
|
if err != nil || info.Mode()&os.ModeSymlink == 0 {
|
|
t.Fatalf("%s must remain a symlink: %v, %v", name, info, err)
|
|
}
|
|
if got, err := os.Readlink(filepath.Join(dst, name)); err != nil || got != want {
|
|
t.Fatalf("%s target = %q, %v; want %q", name, got, err, want)
|
|
}
|
|
}
|
|
}
|
|
assertNoCrossDeviceStage(t, dst)
|
|
})
|
|
|
|
t.Run("regular file root", func(t *testing.T) {
|
|
root := t.TempDir()
|
|
src := filepath.Join(root, "source-file")
|
|
dst := filepath.Join(root, "backup", "source-file")
|
|
if err := os.WriteFile(src, []byte("state"), 0o600); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
forceCrossDeviceRename(t, src, dst)
|
|
if err := moveSkillPathRecoverably(src, dst); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
if got, err := os.ReadFile(dst); err != nil || string(got) != "state" {
|
|
t.Fatalf("backup file = %q, %v", got, err)
|
|
}
|
|
})
|
|
|
|
t.Run("read-only directory mode is restored after copying children", func(t *testing.T) {
|
|
if runtime.GOOS == "windows" {
|
|
t.Skip("POSIX directory permission semantics are unavailable")
|
|
}
|
|
root := t.TempDir()
|
|
t.Cleanup(func() { makeSkillPathTreeWritable(root) })
|
|
src := filepath.Join(root, "external", "dingtalk-chat")
|
|
dst := filepath.Join(root, "backup", "dingtalk-chat")
|
|
if err := os.MkdirAll(src, 0o755); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
if err := os.WriteFile(filepath.Join(src, "SKILL.md"), []byte("read only\n"), 0o444); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
if err := os.Chmod(src, 0o555); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
forceCrossDeviceRename(t, src, dst)
|
|
if err := moveSkillPathRecoverably(src, dst); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
if info, err := os.Stat(dst); err != nil || info.Mode().Perm() != 0o555 {
|
|
t.Fatalf("copied directory mode = %v, %v", info, err)
|
|
}
|
|
if got, err := os.ReadFile(filepath.Join(dst, "SKILL.md")); err != nil || string(got) != "read only\n" {
|
|
t.Fatalf("copied read-only file = %q, %v", got, err)
|
|
}
|
|
})
|
|
|
|
t.Run("rollback uses the same cross-device contract", func(t *testing.T) {
|
|
root := t.TempDir()
|
|
backup := filepath.Join(root, "home", ".dws", "skill-backups", "stamp", "dingtalk-chat")
|
|
original := filepath.Join(root, "external", "claude", "skills", "dingtalk-chat")
|
|
if err := os.MkdirAll(backup, 0o755); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
if err := os.WriteFile(filepath.Join(backup, "SKILL.md"), []byte("old"), 0o644); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
forceCrossDeviceRename(t, backup, original)
|
|
if err := RestoreSkillPath(backup, original); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
if got, err := os.ReadFile(filepath.Join(original, "SKILL.md")); err != nil || string(got) != "old" {
|
|
t.Fatalf("restored content = %q, %v", got, err)
|
|
}
|
|
if _, err := os.Lstat(backup); !os.IsNotExist(err) {
|
|
t.Fatalf("backup must be consumed after verified restore: %v", err)
|
|
}
|
|
})
|
|
}
|
|
|
|
func TestCrossPlatformCoverageSkillPathCrossDeviceFailures(t *testing.T) {
|
|
t.Run("non cross-device rename never copies", func(t *testing.T) {
|
|
src, dst := makeSkillMoveFixture(t)
|
|
copied := false
|
|
testseam.Swap(t, &skillPathRenameNoReplace, func(string, string) (string, error) { return "", os.ErrPermission })
|
|
testseam.Swap(t, &skillPathCopy, func(string, string) error { copied = true; return nil })
|
|
if err := moveSkillPathRecoverably(src, dst); !errors.Is(err, os.ErrPermission) {
|
|
t.Fatalf("error = %v", err)
|
|
}
|
|
if copied {
|
|
t.Fatal("permission failure must not enter cross-device copy fallback")
|
|
}
|
|
assertSkillSourcePreserved(t, src)
|
|
})
|
|
|
|
for _, tc := range []struct {
|
|
name string
|
|
copy func(string, string) error
|
|
verify func(string, string) error
|
|
}{
|
|
{name: "copy failure", copy: func(string, string) error { return errors.New("copy failed") }},
|
|
{name: "verification failure", verify: func(string, string) error { return errors.New("verify failed") }},
|
|
} {
|
|
t.Run(tc.name, func(t *testing.T) {
|
|
src, dst := makeSkillMoveFixture(t)
|
|
forceCrossDeviceRename(t, src, dst)
|
|
if tc.copy != nil {
|
|
testseam.Swap(t, &skillPathCopy, tc.copy)
|
|
}
|
|
if tc.verify != nil {
|
|
testseam.Swap(t, &skillPathVerify, tc.verify)
|
|
}
|
|
if err := moveSkillPathRecoverably(src, dst); err == nil {
|
|
t.Fatal("expected failure")
|
|
}
|
|
assertSkillSourcePreserved(t, src)
|
|
if _, err := os.Lstat(dst); !os.IsNotExist(err) {
|
|
t.Fatalf("formal backup must not publish: %v", err)
|
|
}
|
|
assertNoCrossDeviceStage(t, dst)
|
|
})
|
|
}
|
|
|
|
t.Run("staging publish failure", func(t *testing.T) {
|
|
src, dst := makeSkillMoveFixture(t)
|
|
originalRename := skillPathRenameNoReplace
|
|
testseam.Swap(t, &skillPathRenameNoReplace, func(from, to string) (string, error) {
|
|
if from == src && to == dst {
|
|
return "", testCrossDeviceError()
|
|
}
|
|
if to == dst {
|
|
return "", errors.New("publish failed")
|
|
}
|
|
return originalRename(from, to)
|
|
})
|
|
if err := moveSkillPathRecoverably(src, dst); err == nil || !strings.Contains(err.Error(), "publish failed") {
|
|
t.Fatalf("error = %v", err)
|
|
}
|
|
assertSkillSourcePreserved(t, src)
|
|
assertNoCrossDeviceStage(t, dst)
|
|
})
|
|
|
|
t.Run("published verification failure preserves both", func(t *testing.T) {
|
|
src, dst := makeSkillMoveFixture(t)
|
|
forceCrossDeviceRename(t, src, dst)
|
|
calls := 0
|
|
testseam.Swap(t, &skillPathVerify, func(string, string) error {
|
|
calls++
|
|
if calls == 2 {
|
|
return errors.New("published verify failed")
|
|
}
|
|
return nil
|
|
})
|
|
if err := moveSkillPathRecoverably(src, dst); err == nil || !strings.Contains(err.Error(), "published verify failed") {
|
|
t.Fatalf("error = %v", err)
|
|
}
|
|
assertSkillSourcePreserved(t, src)
|
|
if _, err := os.Lstat(dst); err != nil {
|
|
t.Fatalf("verified backup candidate must remain: %v", err)
|
|
}
|
|
assertNoCrossDeviceStage(t, dst)
|
|
})
|
|
|
|
t.Run("source removal failure preserves both", func(t *testing.T) {
|
|
src, dst := makeSkillMoveFixture(t)
|
|
forceCrossDeviceRename(t, src, dst)
|
|
originalRemove := skillPathRemoveAll
|
|
testseam.Swap(t, &skillPathRemoveAll, func(path string) error {
|
|
if path == src {
|
|
return errors.New("remove failed")
|
|
}
|
|
return originalRemove(path)
|
|
})
|
|
if err := moveSkillPathRecoverably(src, dst); err == nil || !strings.Contains(err.Error(), "均保留") {
|
|
t.Fatalf("error = %v", err)
|
|
}
|
|
assertSkillSourcePreserved(t, src)
|
|
if _, err := os.Lstat(dst); err != nil {
|
|
t.Fatalf("formal backup must remain: %v", err)
|
|
}
|
|
})
|
|
|
|
t.Run("read-only source mode is restored after removal failure", func(t *testing.T) {
|
|
if runtime.GOOS == "windows" {
|
|
t.Skip("POSIX directory permission semantics are unavailable")
|
|
}
|
|
src, dst := makeSkillMoveFixture(t)
|
|
if err := os.Chmod(src, 0o555); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
forceCrossDeviceRename(t, src, dst)
|
|
originalRemove := skillPathRemoveAll
|
|
testseam.Swap(t, &skillPathRemoveAll, func(path string) error {
|
|
if path == src {
|
|
return errors.New("remove failed")
|
|
}
|
|
return originalRemove(path)
|
|
})
|
|
if err := moveSkillPathRecoverably(src, dst); err == nil || !strings.Contains(err.Error(), "均保留") {
|
|
t.Fatalf("error = %v", err)
|
|
}
|
|
if info, err := os.Stat(src); err != nil || info.Mode().Perm() != 0o555 {
|
|
t.Fatalf("source directory mode after failed removal = %v, %v", info, err)
|
|
}
|
|
if err := os.Chmod(src, 0o755); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
})
|
|
|
|
t.Run("source permission preparation failure preserves both", func(t *testing.T) {
|
|
if runtime.GOOS == "windows" {
|
|
t.Skip("POSIX directory permission semantics are unavailable")
|
|
}
|
|
src, dst := makeSkillMoveFixture(t)
|
|
if err := os.Chmod(src, 0o555); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
forceCrossDeviceRename(t, src, dst)
|
|
originalChmod := skillPathChmod
|
|
testseam.Swap(t, &skillPathChmod, func(path string, mode os.FileMode) error {
|
|
if path == src {
|
|
return errors.New("chmod failed")
|
|
}
|
|
return originalChmod(path, mode)
|
|
})
|
|
if err := moveSkillPathRecoverably(src, dst); err == nil || !strings.Contains(err.Error(), "chmod failed") {
|
|
t.Fatalf("error = %v", err)
|
|
}
|
|
if err := originalChmod(src, 0o755); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
})
|
|
|
|
t.Run("lying remover is detected", func(t *testing.T) {
|
|
src, dst := makeSkillMoveFixture(t)
|
|
forceCrossDeviceRename(t, src, dst)
|
|
originalRemove := skillPathRemoveAll
|
|
testseam.Swap(t, &skillPathRemoveAll, func(path string) error {
|
|
if path == src {
|
|
return nil
|
|
}
|
|
return originalRemove(path)
|
|
})
|
|
if err := moveSkillPathRecoverably(src, dst); err == nil || !strings.Contains(err.Error(), "仍存在") {
|
|
t.Fatalf("error = %v", err)
|
|
}
|
|
assertSkillSourcePreserved(t, src)
|
|
})
|
|
|
|
t.Run("same-filesystem fast path", func(t *testing.T) {
|
|
src, dst := makeSkillMoveFixture(t)
|
|
if err := moveSkillPathRecoverably(src, dst); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
if _, err := os.Lstat(src); !os.IsNotExist(err) {
|
|
t.Fatalf("source still exists: %v", err)
|
|
}
|
|
})
|
|
|
|
t.Run("existing destination is never overwritten", func(t *testing.T) {
|
|
src, dst := makeSkillMoveFixture(t)
|
|
if err := os.MkdirAll(dst, 0o755); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
if err := moveSkillPathRecoverably(src, dst); err == nil || !strings.Contains(err.Error(), "目标已存在") {
|
|
t.Fatalf("error = %v", err)
|
|
}
|
|
assertSkillSourcePreserved(t, src)
|
|
})
|
|
}
|
|
|
|
func TestCrossPlatformCoverageVerifySkillPathCopyMismatches(t *testing.T) {
|
|
t.Run("type", func(t *testing.T) {
|
|
root := t.TempDir()
|
|
src, dst := filepath.Join(root, "src"), filepath.Join(root, "dst")
|
|
if err := os.WriteFile(src, []byte("x"), 0o600); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
if err := os.Mkdir(dst, 0o700); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
if err := verifySkillPathCopy(src, dst); err == nil || !strings.Contains(err.Error(), "类型") {
|
|
t.Fatalf("error = %v", err)
|
|
}
|
|
})
|
|
t.Run("directory entries", func(t *testing.T) {
|
|
root := t.TempDir()
|
|
src, dst := filepath.Join(root, "src"), filepath.Join(root, "dst")
|
|
if err := os.Mkdir(src, 0o700); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
if err := os.Mkdir(dst, 0o700); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
if err := os.WriteFile(filepath.Join(src, "one"), []byte("x"), 0o600); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
if err := verifySkillPathCopy(src, dst); err == nil || !strings.Contains(err.Error(), "数量") {
|
|
t.Fatalf("error = %v", err)
|
|
}
|
|
if err := os.WriteFile(filepath.Join(dst, "two"), []byte("x"), 0o600); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
if err := verifySkillPathCopy(src, dst); err == nil || !strings.Contains(err.Error(), "目录项") {
|
|
t.Fatalf("error = %v", err)
|
|
}
|
|
})
|
|
t.Run("file size and digest", func(t *testing.T) {
|
|
root := t.TempDir()
|
|
src, dst := filepath.Join(root, "src"), filepath.Join(root, "dst")
|
|
if err := os.WriteFile(src, []byte("same"), 0o600); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
if err := os.WriteFile(dst, []byte("longer"), 0o600); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
if err := verifySkillPathCopy(src, dst); err == nil || !strings.Contains(err.Error(), "大小") {
|
|
t.Fatalf("error = %v", err)
|
|
}
|
|
if err := os.WriteFile(dst, []byte("diff"), 0o600); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
if err := verifySkillPathCopy(src, dst); err == nil || !strings.Contains(err.Error(), "摘要") {
|
|
t.Fatalf("error = %v", err)
|
|
}
|
|
})
|
|
t.Run("symlink target", func(t *testing.T) {
|
|
root := t.TempDir()
|
|
src, dst := filepath.Join(root, "src"), filepath.Join(root, "dst")
|
|
if err := os.Symlink("one", src); err != nil {
|
|
if runtime.GOOS == "windows" {
|
|
t.Skip("symlink creation unavailable")
|
|
}
|
|
t.Fatal(err)
|
|
}
|
|
if err := os.Symlink("two", dst); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
if err := verifySkillPathCopy(src, dst); err == nil || !strings.Contains(err.Error(), "链接目标") {
|
|
t.Fatalf("error = %v", err)
|
|
}
|
|
})
|
|
}
|
|
|
|
func forceCrossDeviceRename(t *testing.T, src, dst string) {
|
|
t.Helper()
|
|
originalRename := skillPathRenameNoReplace
|
|
testseam.Swap(t, &skillPathRenameNoReplace, func(from, to string) (string, error) {
|
|
if from == src && to == dst {
|
|
return "", testCrossDeviceError()
|
|
}
|
|
return originalRename(from, to)
|
|
})
|
|
}
|
|
|
|
func makeSkillMoveFixture(t *testing.T) (string, string) {
|
|
t.Helper()
|
|
root := t.TempDir()
|
|
t.Cleanup(func() { makeSkillPathTreeWritable(root) })
|
|
src := filepath.Join(root, "external", "dingtalk-chat")
|
|
dst := filepath.Join(root, "home", ".dws", "skill-backups", "stamp", "dingtalk-chat")
|
|
if err := os.MkdirAll(src, 0o755); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
if err := os.WriteFile(filepath.Join(src, "SKILL.md"), []byte("old"), 0o644); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
return src, dst
|
|
}
|
|
|
|
func assertSkillSourcePreserved(t *testing.T, src string) {
|
|
t.Helper()
|
|
if got, err := os.ReadFile(filepath.Join(src, "SKILL.md")); err != nil || string(got) != "old" {
|
|
t.Fatalf("source content = %q, %v", got, err)
|
|
}
|
|
}
|
|
|
|
func assertNoCrossDeviceStage(t *testing.T, dst string) {
|
|
t.Helper()
|
|
entries, err := os.ReadDir(filepath.Dir(dst))
|
|
if err != nil {
|
|
if os.IsNotExist(err) {
|
|
return
|
|
}
|
|
t.Fatal(err)
|
|
}
|
|
prefix := "." + filepath.Base(dst) + ".cross-device-"
|
|
for _, entry := range entries {
|
|
if strings.HasPrefix(entry.Name(), prefix) {
|
|
t.Fatalf("cross-device staging leaked: %s", entry.Name())
|
|
}
|
|
}
|
|
}
|