Use a real Windows lock for the file token store · Entire

Use a real Windows lock for the file token store

0eb86b0→main·

Soph·4w ago·2 files·+39 added/-14 removed

The initial Windows shim made the package compile but its lock was a no-op, so two concurrent file-store writers could lose an update or collide on the shared ".tmp" file. Use LockFileEx/UnlockFileEx (golang.org/x/sys/windows, already a dependency) on the ".lock" file for the same blocking, interprocess advisory exclusion the Unix path gets from flock, which serializes writeFileToken's read-modify-write. Promotes golang.org/x/sys to a direct dependency.

Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com

Sessions

aef01c5e047dView transcript

Changes

2

7 unmodified lines

8
9
10
11
12
13
14
15 unmodified lines

30
31
32
32
33
34

7 unmodified lines

github.com/spf13/cobra v1.10.2
    github.com/stretchr/testify v1.11.1
    github.com/zalando/go-keyring v0.2.8
    golang.org/x/sys v0.44.0
)

require (
15 unmodified lines

golang.org/x/crypto v0.51.0 // indirect
    golang.org/x/net v0.54.0 // indirect
    golang.org/x/sync v0.20.0 // indirect
    golang.org/x/sys v0.44.0 // indirect
    gopkg.in/yaml.v3 v3.0.1 // indirect
)

Mgo.mod+1/-1

4 unmodified lines

5
6
7
8
9
10
11
10
11
12
13
14
15
16
17
18
19
20
12
13
14
15
16
17
22
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
27
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53

4 unmodified lines

import (
    "fmt"
    "os"

"golang.org/x/sys/windows"
)

// Windows has no flock(2). The file token store's writes are atomic
// (temp file + rename), which keeps a single write safe on its own; the lock
// only guards against a lost update between concurrent read-modify-write
// processes, which is rare for a credential store. Rather than pull in a
// Windows-specific locking dependency, open (and create) the lock file so the
// call still succeeds and behaves like a no-op advisory lock.
//
// flockShared / flockExclusive mirror the Unix signatures so callers compile
// unchanged across platforms.
func flockShared(path string) (func(), error)    { return flockOpen(path + ".lock") }
func flockExclusive(path string) (func(), error) { return flockOpen(path + ".lock") }
// Windows has no flock(2); use LockFileEx on a dedicated ".lock" file for the
// same advisory, interprocess mutual exclusion the Unix path gets from flock.
// writeFileToken relies on this to serialize its read-modify-write (and the
// shared temp-file write that precedes the rename), so a no-op would let
// concurrent logins/refreshes lose an update.

func flockOpen(lockPath string) (func(), error) {
// flockShared acquires a shared (read) lock on path+".lock".
func flockShared(path string) (func(), error) {
    return flockOpen(path + ".lock", 0)
}

// flockExclusive acquires an exclusive (write) lock on path+".lock".
func flockExclusive(path string) (func(), error) {
    return flockOpen(path + ".lock", windows.LOCKFILE_EXCLUSIVE_LOCK)
}

func flockOpen(lockPath string, flags uint32) (func(), error) {
    f, err := os.OpenFile(lockPath, os.O_CREATE|os.O_RDWR, 0o600)
    if err != nil {
        return nil, fmt.Errorf("open lock file: %w", err)
    }
    return func() { _ = f.Close() }, nil
    // Lock the entire file range, blocking until the lock is available
    // (no LOCKFILE_FAIL_IMMEDIATELY), matching flock's blocking semantics.
    //
    // os.OpenFile yields a synchronous handle (Go does not pass
    // FILE_FLAG_OVERLAPPED), so LockFileEx blocks until the lock is granted and
    // never returns ERROR_IO_PENDING — that pending/GetOverlappedResult path
    // only applies to handles opened for asynchronous I/O. Treating any error
    // as failure is therefore correct here; this matches the long-standing
    // github.com/gofrs/flock implementation.
    if err := windows.LockFileEx(windows.Handle(f.Fd()), flags, 0, maxUint32, maxUint32, new(windows.Overlapped)); err != nil {
        f.Close()
        return nil, fmt.Errorf("acquire file lock: %w", err)
    }
    return func() {
        //nolint:errcheck // unlock errors on close are not actionable
        windows.UnlockFileEx(windows.Handle(f.Fd()), 0, maxUint32, maxUint32, new(windows.Overlapped))
        f.Close()
    }, nil
}

const maxUint32 = ^uint32(0)