From 2d1670e39051c3b4e59b1a20f6fe682eeaa62646 Mon Sep 17 00:00:00 2001 From: mptyl Date: Tue, 18 Aug 2026 14:30:33 +0200 Subject: [PATCH] fix(windows): complete concurrent auth consumption --- .../tht/internal/safeio/claim_windows_test.go | 59 +++++++++++++++++++ .../internal/safeio/private_root_windows.go | 30 +++++++--- 2 files changed, 80 insertions(+), 9 deletions(-) diff --git a/tools/tht/internal/safeio/claim_windows_test.go b/tools/tht/internal/safeio/claim_windows_test.go index 0cf4632f..a847bdd5 100644 --- a/tools/tht/internal/safeio/claim_windows_test.go +++ b/tools/tht/internal/safeio/claim_windows_test.go @@ -185,3 +185,62 @@ func TestClaimCanonicalPrivateRegularHasOneConcurrentWinner(t *testing.T) { } } } + +func TestCanonicalPrivateClaimConsumeHasOneConcurrentWinner(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) + } + for iteration := range 16 { + source := filepath.Join(parent, fmt.Sprintf("consume-%02d.json", iteration)) + claim := filepath.Join(parent, fmt.Sprintf("consume-%02d.claim", iteration)) + createWindowsPrivateTestFile(t, source, []byte("state")) + + type result struct { + found bool + err error + } + results := make(chan result, 2) + var group sync.WaitGroup + for range 2 { + group.Add(1) + go func() { + defer group.Done() + claimed, err := ClaimCanonicalPrivateRegular(source, claim) + if err != nil || !claimed { + results <- result{err: err} + return + } + contents, found, err := ReadCanonicalPrivateClaim(source, claim, 32) + if err != nil || !found || string(contents) != "state" { + results <- result{err: err} + return + } + removed, err := RemoveCanonicalPrivateClaim(source, claim) + if err != nil || !removed { + results <- result{err: err} + return + } + results <- result{found: true} + }() + } + group.Wait() + close(results) + + winners := 0 + for got := range results { + if got.err != nil { + t.Fatalf("iteration %d concurrent consume error = %v", iteration, got.err) + } + if got.found { + winners++ + } + } + if winners != 1 { + t.Fatalf("iteration %d winning consumes = %d, want 1", iteration, winners) + } + } +} diff --git a/tools/tht/internal/safeio/private_root_windows.go b/tools/tht/internal/safeio/private_root_windows.go index 7d5614f8..1135643d 100644 --- a/tools/tht/internal/safeio/private_root_windows.go +++ b/tools/tht/internal/safeio/private_root_windows.go @@ -955,14 +955,14 @@ func windowsRelativeClaimAbsentOrOrphan(directory *windowsPrivateDirectory, sour const windowsClaimSharingRetries = 100 -func openWindowsPrivateClaimSource(directory windows.Handle, source string) (*windowsPrivateRegularAt, error) { +func openWindowsPrivateClaimRegular( + directory windows.Handle, + name string, + access uint32, + allowedLinks uint32, +) (*windowsPrivateRegularAt, error) { for attempt := 0; ; attempt++ { - value, err := openWindowsPrivateRegularAt( - directory, - source, - windows.FILE_GENERIC_READ|windows.FILE_GENERIC_WRITE|windows.DELETE, - 1, - ) + value, err := openWindowsPrivateRegularAt(directory, name, access, allowedLinks) if !errors.Is(err, windows.ERROR_SHARING_VIOLATION) && !errors.Is(err, windows.STATUS_SHARING_VIOLATION) { return value, err @@ -981,7 +981,12 @@ func (directory *windowsPrivateDirectory) ClaimRegular(source, claim string) (bo // A concurrent winner temporarily holds the source with DELETE access and deliberately // without FILE_SHARE_DELETE. Wait only for that specific, bounded contention before // observing the resulting pair or absence below. Persistent sharing remains unsafe. - value, err := openWindowsPrivateClaimSource(directory.handle, source) + value, err := openWindowsPrivateClaimRegular( + directory.handle, + source, + windows.FILE_GENERIC_READ|windows.FILE_GENERIC_WRITE|windows.DELETE, + 1, + ) if err != nil { pair, pairErr := windowsRelativeClaimPairExists(directory, source, claim) if pairErr == nil && pair { @@ -1058,7 +1063,14 @@ func (directory *windowsPrivateDirectory) RemoveClaim(source, claim string) (boo if directory.Validate() != nil || !validPrivateLeafName(source) || !validPrivateLeafName(claim) { return false, ErrUnsafeFile } - value, err := openWindowsPrivateRegularAt(directory.handle, source, windows.FILE_GENERIC_READ|windows.DELETE, 2) + // A losing claimer can still be closing its no-delete source handle after observing the + // existing hard-link pair. Treat only that bounded sharing window as contention. + value, err := openWindowsPrivateClaimRegular( + directory.handle, + source, + windows.FILE_GENERIC_READ|windows.DELETE, + 2, + ) if err != nil { orphan, orphanErr := windowsRelativeClaimAbsentOrOrphan(directory, source, claim) if orphanErr == nil && orphan {