fix(auth): remove Windows claims by retained handle

2026-08-18 11:04:21 +02:00
parent 5f9a3ae066
commit 0d8e707533
2 changed files with 154 additions and 34 deletions
@@ -123,41 +123,24 @@ func readCanonicalPrivateClaim(source, claim string, maximum int64) ([]byte, boo
 	return contents, true, nil
 }

-func removeCanonicalPrivateClaim(source, claim string) (bool, error) {
-	sourceFile, err := openWindowsPrivateRegular(source, 2)
-	if err != nil {
-		if isWindowsNotFound(err) && windowsPrivateClaimAbsentOrOrphan(claim) {
-			return false, nil
+func removeCanonicalPrivateClaim(source, claim string) (removed bool, resultErr error) {
+	parentPath := filepath.Dir(source)
+	sourceName := filepath.Base(source)
+	claimName := filepath.Base(claim)
+	if parentPath != filepath.Dir(claim) || !validPrivateLeafName(sourceName) || !validPrivateLeafName(claimName) {
+		return false, ErrUnsafeFile
+	}
+	directory, found, err := OpenPrivateDirectory(parentPath, false)
+	if err != nil || !found {
+		return false, ErrUnsafeFile
+	}
+	defer func() {
+		if closeErr := directory.Close(); closeErr != nil && resultErr == nil {
+			resultErr = ErrUnsafeFile
 		}
-		return false, ErrUnsafeFile
-	}
-	claimFile, err := openWindowsPrivateRegular(claim, 2)
-	if err != nil {
-		sourceFile.Close()
-		if isWindowsNotFound(err) {
-			return false, nil
-		}
-		return false, ErrUnsafeFile
-	}
-	if !sameWindowsPrivateFile(sourceFile.info, claimFile.info) {
-		sourceFile.Close()
-		claimFile.Close()
-		return false, ErrUnsafeFile
-	}
-	sourceFile.Close()
-	claimFile.Close()
-	if err := windows.DeleteFile(windows.StringToUTF16Ptr(source)); err != nil {
-		return false, ErrUnsafeFile
-	}
-	remaining, err := openWindowsPrivateRegular(claim, 1)
-	if err != nil {
-		return false, ErrUnsafeFile
-	}
-	remaining.Close()
-	if err := windows.DeleteFile(windows.StringToUTF16Ptr(claim)); err != nil {
-		return false, ErrUnsafeFile
-	}
-	return true, nil
+	}()
+	NotifyPrivateDirectoryTestHookForTest("after-canonical-private-claim-parent-open")
+	return directory.RemoveClaim(sourceName, claimName)
 }

 func openWindowsPrivateRegularWithAllowedLinks(path string, allowed ...uint32) (*windowsPrivateRegular, error) {
@@ -0,0 +1,137 @@
+//go:build windows
+
+package safeio
+
+import (
+	"errors"
+	"os"
+	"path/filepath"
+	"testing"
+)
+
+func createWindowsPrivateTestFile(t *testing.T, path string, contents []byte) {
+	t.Helper()
+	file, err := CreateCanonicalNewPrivateFile(path)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if _, err := file.Write(contents); err != nil {
+		_ = file.Close()
+		t.Fatal(err)
+	}
+	if err := file.Close(); err != nil {
+		t.Fatal(err)
+	}
+}
+
+func TestRemoveCanonicalPrivateClaimRetainsParentDuringDeletion(t *testing.T) {
+	parent := filepath.Join(t.TempDir(), "claims")
+	if err := os.Mkdir(parent, 0o700); err != nil {
+		t.Fatal(err)
+	}
+	if err := ProtectPrivateDirectory(parent); err != nil {
+		t.Fatal(err)
+	}
+	source := filepath.Join(parent, "state.json")
+	claim := filepath.Join(parent, "state.claim")
+	createWindowsPrivateTestFile(t, source, []byte("state"))
+	if claimed, err := ClaimCanonicalPrivateRegular(source, claim); err != nil || !claimed {
+		t.Fatalf("ClaimCanonicalPrivateRegular() = claimed %v, err %v", claimed, err)
+	}
+
+	outside := filepath.Join(t.TempDir(), "outside")
+	if err := os.Mkdir(outside, 0o700); err != nil {
+		t.Fatal(err)
+	}
+	if err := ProtectPrivateDirectory(outside); err != nil {
+		t.Fatal(err)
+	}
+	sentinel := filepath.Join(outside, "sentinel")
+	createWindowsPrivateTestFile(t, sentinel, []byte("outside-safe"))
+	attemptedSwap := false
+	restoreHook := SetPrivateDirectoryTestHookForTest(func(stage string) {
+		if stage != "after-canonical-private-claim-parent-open" || attemptedSwap {
+			return
+		}
+		attemptedSwap = true
+		if err := os.Rename(parent, parent+"-moved"); err == nil {
+			t.Fatal("claim parent rename succeeded while removal retained its handle")
+		}
+	})
+	defer restoreHook()
+
+	removed, err := RemoveCanonicalPrivateClaim(source, claim)
+	if err != nil || !removed || !attemptedSwap {
+		t.Fatalf("RemoveCanonicalPrivateClaim() = removed %v, attempted %v, err %v", removed, attemptedSwap, err)
+	}
+	for _, path := range []string{source, claim} {
+		if _, err := os.Stat(path); !errors.Is(err, os.ErrNotExist) {
+			t.Fatalf("removed path %q still exists: %v", filepath.Base(path), err)
+		}
+	}
+	if contents, err := os.ReadFile(sentinel); err != nil || string(contents) != "outside-safe" {
+		t.Fatalf("outside sentinel = %q, err %v", contents, err)
+	}
+}
+
+func TestRemoveCanonicalPrivateClaimPreservesOrphan(t *testing.T) {
+	parent := filepath.Join(t.TempDir(), "claims")
+	if err := os.Mkdir(parent, 0o700); err != nil {
+		t.Fatal(err)
+	}
+	if err := ProtectPrivateDirectory(parent); err != nil {
+		t.Fatal(err)
+	}
+	source := filepath.Join(parent, "state.json")
+	claim := filepath.Join(parent, "state.claim")
+	createWindowsPrivateTestFile(t, claim, []byte("orphan"))
+
+	removed, err := RemoveCanonicalPrivateClaim(source, claim)
+	if removed || err != nil {
+		t.Fatalf("RemoveCanonicalPrivateClaim() = removed %v, err %v, want false/nil", removed, err)
+	}
+	if _, err := os.Stat(source); !errors.Is(err, os.ErrNotExist) {
+		t.Fatalf("orphan source unexpectedly exists: %v", err)
+	}
+	if contents, err := os.ReadFile(claim); err != nil || string(contents) != "orphan" {
+		t.Fatalf("orphan claim = %q, err %v", contents, err)
+	}
+}
+
+func TestRemoveCanonicalPrivateClaimRejectsMismatchedTwoLinkFiles(t *testing.T) {
+	parent := filepath.Join(t.TempDir(), "claims")
+	if err := os.Mkdir(parent, 0o700); err != nil {
+		t.Fatal(err)
+	}
+	if err := ProtectPrivateDirectory(parent); err != nil {
+		t.Fatal(err)
+	}
+	source := filepath.Join(parent, "state.json")
+	claim := filepath.Join(parent, "state.claim")
+	createWindowsPrivateTestFile(t, source, []byte("source"))
+	createWindowsPrivateTestFile(t, claim, []byte("claim"))
+	sourceAuxiliary := filepath.Join(parent, "state.json.aux")
+	if err := os.Link(source, sourceAuxiliary); err != nil {
+		t.Fatal(err)
+	}
+	claimAuxiliary := filepath.Join(parent, "state.claim.aux")
+	if err := os.Link(claim, claimAuxiliary); err != nil {
+		t.Fatal(err)
+	}
+
+	removed, err := RemoveCanonicalPrivateClaim(source, claim)
+	if removed || !errors.Is(err, ErrUnsafeFile) {
+		t.Fatalf("RemoveCanonicalPrivateClaim() = removed %v, err %v, want false/ErrUnsafeFile", removed, err)
+	}
+	for path, want := range map[string]string{
+		source:          "source",
+		claim:           "claim",
+		sourceAuxiliary: "source",
+		claimAuxiliary:  "claim",
+	} {
+		contents, readErr := os.ReadFile(path)
+		if readErr != nil || string(contents) != want {
+			t.Fatalf("mismatched pair path %q = %q, err %v", filepath.Base(path), contents, readErr)
+		}
+	}
+}