Serialize MSAL cache updates across processes

go.mod

@@ -2,12 +2,15 @@ module github.com/energyworldnet/az-devops-token
 
 go 1.23
 
-require github.com/AzureAD/microsoft-authentication-library-for-go v1.7.2
+require (
+	github.com/AzureAD/microsoft-authentication-library-for-go v1.7.2
+	github.com/gofrs/flock v0.12.1
+)
 
 require (
 	github.com/golang-jwt/jwt/v5 v5.2.2 // indirect
 	github.com/google/uuid v1.3.0 // indirect
 	github.com/kylelemons/godebug v1.1.0 // indirect
 	github.com/pkg/browser v0.0.0-20210911075715-681adbf594b8 // indirect
-	golang.org/x/sys v0.5.0 // indirect
+	golang.org/x/sys v0.22.0 // indirect
 )

go.sum

@@ -1,5 +1,9 @@
 github.com/AzureAD/microsoft-authentication-library-for-go v1.7.2 h1:RHK7bS+HQMslb1sZpAokUt+zTVmue0hKSs2C791hhzU=
 github.com/AzureAD/microsoft-authentication-library-for-go v1.7.2/go.mod h1:HKpQxkWaGLJ+D/5H8QRpyQXA1eKjxkFlOMwck5+33Jk=
+github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
+github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
+github.com/gofrs/flock v0.12.1 h1:MTLVXXHf8ekldpJk3AKicLij9MdwOWkZ+a/jHHZby9E=
+github.com/gofrs/flock v0.12.1/go.mod h1:9zxTsyu5xtJ9DK+1tFZyibEV7y3uwDxPPfbxeeHCoD0=
 github.com/golang-jwt/jwt/v5 v5.2.2 h1:Rl4B7itRWVtYIHFrSNd7vhTiz9UpLdi6gZhZ3wEeDy8=
 github.com/golang-jwt/jwt/v5 v5.2.2/go.mod h1:pqrtFR0X4osieyHYxtmOUWsAWrfe1Q5UVIyoH402zdk=
 github.com/google/uuid v1.3.0 h1:t6JiXgmwXMjEs8VusXIJk2BXHsn+wx8BZdTaoZ5fu7I=
@@ -8,6 +12,12 @@ github.com/kylelemons/godebug v1.1.0 h1:RPNrshWIDI6G2gRW9EHilWtl7Z6Sb1BR0xunSBf0
 github.com/kylelemons/godebug v1.1.0/go.mod h1:9/0rRGxNHcop5bhtWyNeEfOS8JIWk580+fNqagV/RAw=
 github.com/pkg/browser v0.0.0-20210911075715-681adbf594b8 h1:KoWmjvw+nsYOo29YJK9vDA65RGE3NrOnUtO7a+RF9HU=
 github.com/pkg/browser v0.0.0-20210911075715-681adbf594b8/go.mod h1:HKlIX3XHQyzLZPlr7++PzdhaXEj94dEiJgZDTsxEqUI=
+github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
+github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
+github.com/stretchr/testify v1.9.0 h1:HtqpIVDClZ4nwg75+f6Lvsy/wHu+3BoSGCbBAcpTsTg=
+github.com/stretchr/testify v1.9.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY=
 golang.org/x/sys v0.0.0-20210616045830-e2b7044e8c71/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
-golang.org/x/sys v0.5.0 h1:MUK/U/4lj1t1oPg0HfuXDN/Z1wv31ZJ/YcPiGccS4DU=
-golang.org/x/sys v0.5.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
+golang.org/x/sys v0.22.0 h1:RI27ohtqKCnwULzJLqkv897zojh5/DwS/ENaMzUOaWI=
+golang.org/x/sys v0.22.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
+gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=
+gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=

internal/token/token.go

@@ -18,6 +18,7 @@ import (
 
 	"github.com/AzureAD/microsoft-authentication-library-for-go/apps/cache"
 	"github.com/AzureAD/microsoft-authentication-library-for-go/apps/public"
+	"github.com/gofrs/flock"
 )
 
 // adoResource is the public, global Azure DevOps resource ID (identical for
@@ -32,6 +33,11 @@ const adoResource = "499b84ac-1321-427f-aa17-267ca6975798"
 // feed tokens live ~1h, so this leaves ample refresh headroom.
 const MinLifetime = 20 * time.Minute
 
+// lockRetryDelay is how long WithLock sleeps between attempts to acquire a
+// contended cross-process lock. Operations under the lock are sub-second, so a
+// short poll keeps a waiter's latency low without busy-spinning.
+const lockRetryDelay = 50 * time.Millisecond
+
 // Scopes returns the delegated scopes requested for a feed token.
 func Scopes() []string { return []string{adoResource + "/vso.packaging"} }
 
@@ -81,6 +87,48 @@ func (c *FileCache) Export(_ context.Context, m cache.Marshaler, _ cache.ExportH
 	return os.Rename(tmp, c.Path)
 }
 
+// WithLock runs fn while holding an exclusive cross-process advisory lock on
+// lockPath, then releases it. It serializes the whole reload → refresh →
+// export sequence across concurrent invocations so a later writer's Export
+// cannot clobber a newer refresh-token/cache update a peer just wrote. The
+// lock is an OS advisory lock, so it is released automatically if the process
+// dies mid-operation — a crash leaves no stale lock.
+//
+// It blocks (polling every lockRetryDelay) until the lock is acquired or ctx is
+// done. When ctx expires first it returns (false, nil) without running fn, so a
+// caller on a latency-sensitive path can degrade rather than hang. A genuine
+// lock error (e.g. the lock directory cannot be created) is returned as-is.
+func WithLock(ctx context.Context, lockPath string, fn func()) (bool, error) {
+	/* The lock is acquired before Export creates the cache directory, so
+	   ensure the parent exists first (mirrors Export). Otherwise the first run
+	   on a machine with a not-yet-created cache dir couldn't create the
+	   lockfile. */
+	if err := os.MkdirAll(filepath.Dir(lockPath), 0o700); err != nil {
+		return false, err
+	}
+	fl := flock.New(lockPath)
+	locked, err := fl.TryLockContext(ctx, lockRetryDelay)
+	if err != nil {
+		/* A context error means we simply ran out of time waiting for a peer to
+		   release — report it as a clean give-up (false, nil) so the caller can
+		   degrade, not as a failure. */
+		if errors.Is(err, context.DeadlineExceeded) || errors.Is(err, context.Canceled) {
+			return false, nil
+		}
+		return false, err
+	}
+	if !locked {
+		return false, nil
+	}
+	/* Release is best-effort: an Unlock failure is not actionable here — this is
+	   a short-lived CLI about to exit, and the OS releases the advisory lock and
+	   closes the descriptor on exit regardless. Folding it into the return would
+	   also make a rare release hiccup discard an already-acquired token. */
+	defer func() { _ = fl.Unlock() }()
+	fn()
+	return true, nil
+}
+
 // StripAccessTokens removes the access-token entries from a serialized MSAL
 // cache, leaving the refresh token and everything else intact. MSAL's silent
 // path then finds no cached access token and redeems the refresh token for a

internal/token/token_test.go

@@ -1,7 +1,14 @@
 package token_test
 
 import (
+	"bufio"
+	"context"
 	"encoding/json"
+	"fmt"
+	"io"
+	"os"
+	"os/exec"
+	"path/filepath"
 	"testing"
 	"time"
 
@@ -42,6 +49,152 @@ func TestStripAccessTokensRejectsInvalidJSON(t *testing.T) {
 	}
 }
 
+func TestWithLockRunsFnAndReleases(t *testing.T) {
+	lockPath := filepath.Join(t.TempDir(), "cache.lock")
+
+	ran := false
+	locked, err := token.WithLock(context.Background(), lockPath, func() { ran = true })
+	if err != nil {
+		t.Fatalf("WithLock: %v", err)
+	}
+	if !locked {
+		t.Fatal("expected to acquire an uncontended lock")
+	}
+	if !ran {
+		t.Error("fn should have run while the lock was held")
+	}
+
+	// The lock must be released on return: a second acquisition succeeds.
+	locked, err = token.WithLock(context.Background(), lockPath, func() {})
+	if err != nil {
+		t.Fatalf("WithLock (second): %v", err)
+	}
+	if !locked {
+		t.Error("lock should be released after WithLock returns")
+	}
+}
+
+func TestWithLockCreatesParentDir(t *testing.T) {
+	// A not-yet-created cache dir (the pre-login state) must not stop the lock.
+	lockPath := filepath.Join(t.TempDir(), "nested", "cache.lock")
+	locked, err := token.WithLock(context.Background(), lockPath, func() {})
+	if err != nil {
+		t.Fatalf("WithLock: %v", err)
+	}
+	if !locked {
+		t.Error("expected to acquire the lock after creating the parent dir")
+	}
+}
+
+// lockHolderEnv names the env var that switches TestWithLockCrossProcess into
+// its child (lock-holder) role. The child holds the lock on the named path
+// until the parent closes its stdin.
+const lockHolderEnv = "AZ_DEVOPS_TOKEN_TEST_LOCK_PATH"
+
+// lockHeldSignal is written to stdout by the child once the lock is held, so
+// the parent only starts contending after acquisition.
+const lockHeldSignal = "LOCK_HELD"
+
+func TestWithLockCrossProcess(t *testing.T) {
+	if lockPath := os.Getenv(lockHolderEnv); lockPath != "" {
+		// Child role: hold the lock until the parent closes our stdin (EOF),
+		// signalling readiness first.
+		_, _ = token.WithLock(context.Background(), lockPath, func() {
+			fmt.Println(lockHeldSignal)
+			_, _ = io.Copy(io.Discard, os.Stdin)
+		})
+		return
+	}
+
+	lockPath := filepath.Join(t.TempDir(), "cache.lock")
+
+	// Re-exec this test binary as a separate process that holds the lock, to
+	// exercise the genuinely cross-process advisory lock rather than same-process
+	// scheduling. The context stops the helper if the test exits early.
+	holderCtx, cancelHolder := context.WithCancel(context.Background())
+	cmd := exec.CommandContext(holderCtx, os.Args[0], "-test.run=^TestWithLockCrossProcess$") //nolint:gosec // re-execs this test binary itself, not external input
+	cmd.Env = append(os.Environ(), lockHolderEnv+"="+lockPath)
+	cmd.Stderr = os.Stderr
+	stdin, err := cmd.StdinPipe()
+	if err != nil {
+		t.Fatalf("StdinPipe: %v", err)
+	}
+	stdout, err := cmd.StdoutPipe()
+	if err != nil {
+		t.Fatalf("StdoutPipe: %v", err)
+	}
+	if err := cmd.Start(); err != nil {
+		t.Fatalf("start lock holder: %v", err)
+	}
+	// Safety net: if an assertion below fails early, don't leave the child
+	// blocked (and the lockfile undeletable).
+	t.Cleanup(func() {
+		cancelHolder()
+		_ = cmd.Wait()
+	})
+
+	// Wait until the child reports the lock is held, bounding the wait so a
+	// stalled child (never signals) or one that exits before signalling (EOF)
+	// fails fast with a clear diagnostic instead of hanging or falling through
+	// to a misleading contended-lock assertion.
+	ready := make(chan error, 1)
+	go func() {
+		scanner := bufio.NewScanner(stdout)
+		for scanner.Scan() {
+			if scanner.Text() == lockHeldSignal {
+				ready <- nil
+				return
+			}
+		}
+		if err := scanner.Err(); err != nil {
+			ready <- fmt.Errorf("reading lock holder output: %w", err)
+			return
+		}
+		ready <- fmt.Errorf("lock holder exited before signalling it held the lock")
+	}()
+	select {
+	case err := <-ready:
+		if err != nil {
+			t.Fatalf("waiting for lock holder: %v", err)
+		}
+	case <-time.After(10 * time.Second):
+		t.Fatal("timed out waiting for lock holder to acquire the lock")
+	}
+
+	// The lock is held by another process: acquisition must give up within the
+	// deadline without running fn, so a latency-sensitive caller can degrade.
+	ctx, cancel := context.WithTimeout(context.Background(), 50*time.Millisecond)
+	defer cancel()
+	ran := false
+	locked, err := token.WithLock(ctx, lockPath, func() { ran = true })
+	if err != nil {
+		t.Fatalf("contended WithLock: %v", err)
+	}
+	if locked {
+		t.Error("should not acquire a lock another process holds")
+	}
+	if ran {
+		t.Error("fn must not run when the lock is not acquired")
+	}
+
+	// Release the holder and wait for it to exit, so the lock is fully released.
+	if err := stdin.Close(); err != nil {
+		t.Fatalf("close holder stdin: %v", err)
+	}
+	if err := cmd.Wait(); err != nil {
+		t.Fatalf("lock holder exited with error: %v", err)
+	}
+
+	// The lock is acquirable again now that the holder is gone.
+	locked, err = token.WithLock(context.Background(), lockPath, func() {})
+	if err != nil {
+		t.Fatalf("post-release WithLock: %v", err)
+	}
+	if !locked {
+		t.Error("lock should be acquirable after the holder exits")
+	}
+}
+
 func TestNeedsForcedRefresh(t *testing.T) {
 	now := time.Unix(1_700_000_000, 0)
 	cases := []struct {

main.go

@@ -97,29 +97,77 @@ func main() {
 		return
 	}
 
-	// A cached/refreshable token satisfies both paths: it's what `print` emits,
-	// and it makes `login` idempotent (no prompt when already signed in).
-	silentCtx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
-	tok, ok := token.Silent(silentCtx, app, fc)
-	cancel()
-	if ok {
-		if login {
-			fmt.Fprintln(os.Stderr, "already signed in")
+	// Serialize the whole acquire across processes so a peer's reload → refresh →
+	// export can't interleave with ours and clobber a newer cache write. The lock
+	// is held from the silent attempt through any interactive sign-in, so two
+	// logins can't both prompt and race their writes; it also dedupes concurrent
+	// refreshes, since a waiter finds the fresh token already cached on release.
+	lockPath := cachePath() + ".lock"
+
+	// Bound the wait for the lock. `print` shares mise's tolerance for a slow
+	// config load; `login` may queue behind an in-progress interactive sign-in,
+	// which is user-paced and can take a while.
+	acquireTimeout := 30 * time.Second
+	if login {
+		acquireTimeout = 2 * time.Minute
+	}
+	lockCtx, cancelLock := context.WithTimeout(context.Background(), acquireTimeout)
+	defer cancelLock()
+
+	var (
+		tok       string
+		haveToken bool
+		loginErr  error
+	)
+	locked, lockErr := token.WithLock(lockCtx, lockPath, func() {
+		// A cached/refreshable token satisfies both paths: it's what `print` emits,
+		// and it makes `login` idempotent (no prompt when already signed in).
+		silentCtx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
+		tok, haveToken = token.Silent(silentCtx, app, fc)
+		cancel()
+		if haveToken || !login {
 			return
 		}
-		fmt.Print(tok)
+		// No cached token on the login path: sign in interactively, still under the
+		// lock. context.Background() (not lockCtx) bounds this — the browser round
+		// trip must not be cut short by the acquire timeout.
+		loginErr = interactiveLogin(context.Background(), app)
+	})
+	if lockErr != nil {
+		fmt.Fprintf(os.Stderr, "cache lock: %v\n", lockErr)
+		if login {
+			os.Exit(1)
+		}
+		return
+	}
+	if !locked {
+		// A peer held the lock past our budget. On `print`, stay silent (empty
+		// output, exit 0) so mise's config load falls through to a clean 401; on
+		// `login`, surface it.
+		if login {
+			fmt.Fprintln(os.Stderr, "timed out waiting for another sign-in to finish")
+			os.Exit(1)
+		}
 		return
 	}
 
 	if !login {
-		// No usable token — degrade to empty output so mise's config load and the
-		// consuming package manager fall through to a clean 401 rather than erroring.
-		fmt.Fprintln(os.Stderr, "no token — run `az-devops-token login`")
+		if !haveToken {
+			// No usable token — degrade to empty output so mise's config load and the
+			// consuming package manager fall through to a clean 401 rather than erroring.
+			fmt.Fprintln(os.Stderr, "no token — run `az-devops-token login`")
+			return
+		}
+		fmt.Print(tok)
 		return
 	}
 
-	if err := interactiveLogin(context.Background(), app); err != nil {
-		fmt.Fprintf(os.Stderr, "login failed: %v\n", err)
+	if haveToken {
+		fmt.Fprintln(os.Stderr, "already signed in")
+		return
+	}
+	if loginErr != nil {
+		fmt.Fprintf(os.Stderr, "login failed: %v\n", loginErr)
 		os.Exit(1)
 	}
 	fmt.Fprintln(os.Stderr, "signed in")