From 0db91cfc44ed56f7cf0ce655ff926d2dcf5d0e47 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=8E=89=E6=BE=9C?= Date: Mon, 17 Aug 2026 14:03:28 +0800 Subject: [PATCH] fix(skills): prove Windows rollback identity via stable file ID NTFS file tunneling can restore the original creation time for a recreated same-named object, which defeated the creation-time incarnation check and allowed rollback to delete a concurrent replacement. Replace the platform-specific identity pair with a single skillPathIdentityProven function: - Unix: delegates to os.SameFile (inode/dev), ignoring file ID strings - Windows: compares VolumeSerialNumber:FileIndexHigh:FileIndexLow from GetFileInformationByHandle, which uniquely identifies the file on the volume for its lifetime and is unaffected by tunneling When the file ID cannot be obtained at publish time, identity is not proven and the auto-delete is refused. Add a regression test that simulates tunneled creation time and verifies rollback still refuses the concurrent replacement. --- internal/upgrade/skill_path_move.go | 2 + internal/upgrade/skill_publication.go | 11 ++-- .../skill_publication_identity_darwin.go | 10 +++- .../skill_publication_identity_linux.go | 10 +++- .../skill_publication_identity_other.go | 10 +++- .../skill_publication_identity_windows.go | 54 +++++++++++++++---- internal/upgrade/skill_publication_test.go | 25 +++++++++ 7 files changed, 107 insertions(+), 15 deletions(-) diff --git a/internal/upgrade/skill_path_move.go b/internal/upgrade/skill_path_move.go index b1bb95f7..0c8acbd9 100644 --- a/internal/upgrade/skill_path_move.go +++ b/internal/upgrade/skill_path_move.go @@ -24,6 +24,8 @@ var ( skillPathLink = os.Link skillPathRemove = os.Remove skillPathReadDir = os.ReadDir + skillPathFileIdentity = skillPathFileIdentityImpl + skillPathFileIncarnation = skillPathFileIncarnationImpl skillPathReadlink = os.Readlink skillPathSymlink = os.Symlink skillPathOpen = os.Open diff --git a/internal/upgrade/skill_publication.go b/internal/upgrade/skill_publication.go index 0bd9ecdb..6926279c 100644 --- a/internal/upgrade/skill_publication.go +++ b/internal/upgrade/skill_publication.go @@ -18,6 +18,7 @@ type SkillPathPublication struct { fingerprint [sha256.Size]byte identity os.FileInfo incarnation string + fileID string } // PublishSkillPathNoReplace atomically publishes a staged path without ever @@ -31,6 +32,7 @@ func PublishSkillPathNoReplace(staged, destination string) (SkillPathPublication if err != nil { return SkillPathPublication{}, fmt.Errorf("计算待发布 Skill 身份失败 %s: %w", staged, err) } + stagedFileID := skillPathFileIdentity(staged) if err := skillPathRenameNoReplace(staged, destination); err != nil { return SkillPathPublication{}, fmt.Errorf("目标必须不存在的 Skill 发布失败 %s: %w", destination, err) } @@ -42,7 +44,8 @@ func PublishSkillPathNoReplace(staged, destination string) (SkillPathPublication if err != nil { return SkillPathPublication{}, fmt.Errorf("确认已发布 Skill 内容失败 %s（对象保留）: %w", destination, err) } - if !skillPathSameFileIdentity(identity, publishedIdentity) || publishedFingerprint != fingerprint { + publishedFileID := skillPathFileIdentity(destination) + if !skillPathIdentityProven(identity, publishedIdentity, stagedFileID, publishedFileID) || publishedFingerprint != fingerprint { return SkillPathPublication{}, fmt.Errorf("确认已发布 Skill 身份失败 %s（对象保留）: staging 身份已变化", destination) } return SkillPathPublication{ @@ -50,6 +53,7 @@ func PublishSkillPathNoReplace(staged, destination string) (SkillPathPublication fingerprint: fingerprint, identity: publishedIdentity, incarnation: skillPathFileIncarnation(publishedIdentity), + fileID: publishedFileID, }, nil } @@ -93,7 +97,8 @@ func rollbackSkillPathPublication(publication SkillPathPublication) (err error) cleanupRoot(), ) } - if publication.identity == nil || !skillPathSameFileIdentity(publication.identity, liveIdentity) || + if publication.identity == nil || + !skillPathIdentityProven(publication.identity, liveIdentity, publication.fileID, skillPathFileIdentity(destination)) || publication.incarnation != skillPathFileIncarnation(liveIdentity) || liveFingerprint != publication.fingerprint { return errors.Join( @@ -113,7 +118,7 @@ func rollbackSkillPathPublication(publication SkillPathPublication) (err error) actualIdentity, identityErr := skillPathLstat(quarantine) actual, fingerprintErr := fingerprintSkillPath(quarantine) if identityErr == nil && fingerprintErr == nil && - skillPathSameFileIdentity(publication.identity, actualIdentity) && + skillPathIdentityProven(publication.identity, actualIdentity, publication.fileID, skillPathFileIdentity(quarantine)) && actual == publication.fingerprint { if removeErr := removePublishedSkillSource(quarantine); removeErr != nil { return fmt.Errorf("移除事务发布的 Skill 失败 %s（隔离于 %s）: %w", destination, quarantine, removeErr) diff --git a/internal/upgrade/skill_publication_identity_darwin.go b/internal/upgrade/skill_publication_identity_darwin.go index a3547e4c..195f317b 100644 --- a/internal/upgrade/skill_publication_identity_darwin.go +++ b/internal/upgrade/skill_publication_identity_darwin.go @@ -8,7 +8,7 @@ import ( "syscall" ) -func skillPathFileIncarnation(info os.FileInfo) string { +func skillPathFileIncarnationImpl(info os.FileInfo) string { stat, ok := info.Sys().(*syscall.Stat_t) if !ok { return fmt.Sprintf("unknown:%T", info.Sys()) @@ -20,3 +20,11 @@ func skillPathFileIncarnation(info os.FileInfo) string { func skillPathSameFileIdentity(left, right os.FileInfo) bool { return os.SameFile(left, right) } + +func skillPathFileIdentityImpl(_ string) string { + return "" +} + +func skillPathIdentityProven(staged, published os.FileInfo, _, _ string) bool { + return skillPathSameFileIdentity(staged, published) +} diff --git a/internal/upgrade/skill_publication_identity_linux.go b/internal/upgrade/skill_publication_identity_linux.go index cb62e1e1..6e350aab 100644 --- a/internal/upgrade/skill_publication_identity_linux.go +++ b/internal/upgrade/skill_publication_identity_linux.go @@ -8,7 +8,7 @@ import ( "syscall" ) -func skillPathFileIncarnation(info os.FileInfo) string { +func skillPathFileIncarnationImpl(info os.FileInfo) string { stat, ok := info.Sys().(*syscall.Stat_t) if !ok { return fmt.Sprintf("unknown:%T", info.Sys()) @@ -19,3 +19,11 @@ func skillPathFileIncarnation(info os.FileInfo) string { func skillPathSameFileIdentity(left, right os.FileInfo) bool { return os.SameFile(left, right) } + +func skillPathFileIdentityImpl(_ string) string { + return "" +} + +func skillPathIdentityProven(staged, published os.FileInfo, _, _ string) bool { + return skillPathSameFileIdentity(staged, published) +} diff --git a/internal/upgrade/skill_publication_identity_other.go b/internal/upgrade/skill_publication_identity_other.go index 53c0bc86..d487c195 100644 --- a/internal/upgrade/skill_publication_identity_other.go +++ b/internal/upgrade/skill_publication_identity_other.go @@ -7,10 +7,18 @@ import ( "os" ) -func skillPathFileIncarnation(info os.FileInfo) string { +func skillPathFileIncarnationImpl(info os.FileInfo) string { return fmt.Sprintf("%d:%d:%s", info.ModTime().UnixNano(), info.Size(), info.Mode()) } func skillPathSameFileIdentity(left, right os.FileInfo) bool { return os.SameFile(left, right) } + +func skillPathFileIdentityImpl(_ string) string { + return "" +} + +func skillPathIdentityProven(staged, published os.FileInfo, _, _ string) bool { + return skillPathSameFileIdentity(staged, published) +} diff --git a/internal/upgrade/skill_publication_identity_windows.go b/internal/upgrade/skill_publication_identity_windows.go index 107e266a..7e3d409c 100644 --- a/internal/upgrade/skill_publication_identity_windows.go +++ b/internal/upgrade/skill_publication_identity_windows.go @@ -6,9 +6,11 @@ import ( "fmt" "os" "syscall" + + "golang.org/x/sys/windows" ) -func skillPathFileIncarnation(info os.FileInfo) string { +func skillPathFileIncarnationImpl(info os.FileInfo) string { stat, ok := info.Sys().(*syscall.Win32FileAttributeData) if !ok { return fmt.Sprintf("unknown:%T", info.Sys()) @@ -16,13 +18,47 @@ func skillPathFileIncarnation(info os.FileInfo) string { return fmt.Sprintf("%d:%d", stat.CreationTime.HighDateTime, stat.CreationTime.LowDateTime) } +// skillPathSameFileIdentity returns false because os.FileInfo on Windows does +// not expose the stable file ID (VolumeSerialNumber / FileIndex from +// GetFileInformationByHandle), which is the only primitive that distinguishes +// a recreated file from the original after NTFS file tunneling restores the +// creation time. Callers must use skillPathIdentityProven, which compares file +// IDs obtained via GetFileInformationByHandle. func skillPathSameFileIdentity(_, _ os.FileInfo) bool { - // MoveFile can surface different metadata identities for the same reparse - // point before and after rename, so Windows ownership relies on the - // separately checked creation-time incarnation plus the lexical fingerprint. - // That pair is strong evidence but not proof: NTFS file tunneling restores - // the original creation time when a same-named object is recreated in the - // same directory shortly afterwards, so a byte-identical concurrent - // replacement can still be attributed to this transaction. - return true + return false +} + +// skillPathFileIdentityImpl queries the stable volume serial number and file +// index via GetFileInformationByHandle. Unlike creation time (which NTFS file +// tunneling can restore for a recreated same-named object), the file index +// uniquely identifies the file on the volume for its lifetime. +func skillPathFileIdentityImpl(path string) string { + p, err := windows.UTF16PtrFromString(path) + if err != nil { + return "" + } + handle, err := windows.CreateFile( + p, windows.FILE_READ_ATTRIBUTES, + windows.FILE_SHARE_READ|windows.FILE_SHARE_WRITE|windows.FILE_SHARE_DELETE, + nil, windows.OPEN_EXISTING, windows.FILE_FLAG_BACKUP_SEMANTICS, 0, + ) + if err != nil { + return "" + } + defer windows.CloseHandle(handle) + var info windows.ByHandleFileInformation + if err := windows.GetFileInformationByHandle(handle, &info); err != nil { + return "" + } + return fmt.Sprintf("%d:%d:%d", info.VolumeSerialNumber, info.FileIndexHigh, info.FileIndexLow) +} + +// skillPathIdentityProven is the real identity proof on Windows. An empty +// expected value means the file ID could not be obtained at publish time, so +// identity cannot be proven and the caller must refuse the auto-delete. +func skillPathIdentityProven(_, _ os.FileInfo, expected, actual string) bool { + if expected == "" { + return false + } + return expected == actual } diff --git a/internal/upgrade/skill_publication_test.go b/internal/upgrade/skill_publication_test.go index a5c3eb38..e69387b0 100644 --- a/internal/upgrade/skill_publication_test.go +++ b/internal/upgrade/skill_publication_test.go @@ -65,6 +65,31 @@ func TestCrossPlatformCoverageSkillPublicationNoClobberAndOwnedRollback(t *testi } assertUpgradeSkillContent(t, destination, "new") }) + + t.Run("tunneled replacement with same creation time is still refused", func(t *testing.T) { + root := t.TempDir() + staged := filepath.Join(root, "staged") + destination := filepath.Join(root, "destination") + seedUpgradeSkill(t, staged, "new", false) + // NTFS file tunneling can restore the original creation time for a + // same-named recreation, defeating the incarnation check. The file ID + // (or inode on Unix) must still differ and block the rollback. + testseam.Swap(t, &skillPathFileIncarnation, func(os.FileInfo) string { return "tunneled" }) + publication, err := PublishSkillPathNoReplace(staged, destination) + if err != nil { + t.Fatal(err) + } + if err := os.RemoveAll(destination); err != nil { + t.Fatal(err) + } + seedUpgradeSkill(t, destination, "new", false) + + err = RollbackSkillPathPublications([]SkillPathPublication{publication}) + if err == nil || !strings.Contains(err.Error(), "拒绝删除非本事务") { + t.Fatalf("tunneled replacement rollback error = %v", err) + } + assertUpgradeSkillContent(t, destination, "new") + }) } func TestCrossPlatformCoverageSkillPublicationFailureEdges(t *testing.T) {