fix(tokenstore): add mutex to UnsafeStore.Revoke to prevent concurrent revocation loss

commit: 8197cddd42a3f9b4da49a06c26246228d7863a0c
author: bnema · 8mo ago · 2026-02-25 01:33 UTC
parent: cdeffe2
signature: unsigned

2 files · +79 -0

M internal/adapters/out/tokenstore/unsafe.go +8 -0


@@ -9,6 +9,7 @@ import (
9	9		"os"
10	10		"path/filepath"
11	11		"strings"
	12	+	"sync"
12	13
13	14		"github.com/bnema/zerowrap"
14	15

@@ -32,6 +33,7 @@ const (
32	33		// UnsafeStore implements TokenStore using plain text files.
33	34		// WARNING: This store does not encrypt secrets. Only use when pass/sops are unavailable.
34	35		type UnsafeStore struct {
	36	+	mu sync.RWMutex
35	37		dataDir string
36	38		log zerowrap.Logger
37	39		}

@@ -187,6 +189,9 @@ func (s *UnsafeStore) ListTokens(_ context.Context) ([]domain.Token, error) {
187	189
188	190		// Revoke adds token ID to revocation list file.
189	191		func (s *UnsafeStore) Revoke(_ context.Context, tokenID string) error {
	192	+	s.mu.Lock()
	193	+	defer s.mu.Unlock()
	194	+
190	195		revokedFile := filepath.Join(s.dataDir, unsafeRevokedFile)
191	196
192	197		// Ensure parent directory exists

@@ -229,6 +234,9 @@ func (s *UnsafeStore) Revoke(_ context.Context, tokenID string) error {
229	234
230	235		// IsRevoked checks if token ID is in revocation list.
231	236		func (s *UnsafeStore) IsRevoked(_ context.Context, tokenID string) (bool, error) {
	237	+	s.mu.RLock()
	238	+	defer s.mu.RUnlock()
	239	+
232	240		revokedList, err := s.getRevokedList()
233	241		if err != nil {
234	242		return false, err


@@ -9,6 +9,7 @@ import (
9	"os"	9		"os"
10	"path/filepath"	10		"path/filepath"
11	"strings"	11		"strings"
		12	+	"sync"
12		13
13	"github.com/bnema/zerowrap"	14		"github.com/bnema/zerowrap"
14		15

@@ -32,6 +33,7 @@ const (
32	// UnsafeStore implements TokenStore using plain text files.	33		// UnsafeStore implements TokenStore using plain text files.
33	// WARNING: This store does not encrypt secrets. Only use when pass/sops are unavailable.	34		// WARNING: This store does not encrypt secrets. Only use when pass/sops are unavailable.
34	type UnsafeStore struct {	35		type UnsafeStore struct {
		36	+	mu sync.RWMutex
35	dataDir string	37		dataDir string
36	log zerowrap.Logger	38		log zerowrap.Logger
37	}	39		}

@@ -187,6 +189,9 @@ func (s *UnsafeStore) ListTokens(_ context.Context) ([]domain.Token, error) {
187		189
188	// Revoke adds token ID to revocation list file.	190		// Revoke adds token ID to revocation list file.
189	func (s *UnsafeStore) Revoke(_ context.Context, tokenID string) error {	191		func (s *UnsafeStore) Revoke(_ context.Context, tokenID string) error {
		192	+	s.mu.Lock()
		193	+	defer s.mu.Unlock()
		194	+
190	revokedFile := filepath.Join(s.dataDir, unsafeRevokedFile)	195		revokedFile := filepath.Join(s.dataDir, unsafeRevokedFile)
191		196
192	// Ensure parent directory exists	197		// Ensure parent directory exists

@@ -229,6 +234,9 @@ func (s *UnsafeStore) Revoke(_ context.Context, tokenID string) error {
229		234
230	// IsRevoked checks if token ID is in revocation list.	235		// IsRevoked checks if token ID is in revocation list.
231	func (s *UnsafeStore) IsRevoked(_ context.Context, tokenID string) (bool, error) {	236		func (s *UnsafeStore) IsRevoked(_ context.Context, tokenID string) (bool, error) {
		237	+	s.mu.RLock()
		238	+	defer s.mu.RUnlock()
		239	+
232	revokedList, err := s.getRevokedList()	240		revokedList, err := s.getRevokedList()
233	if err != nil {	241		if err != nil {
234	return false, err	242		return false, err

A internal/adapters/out/tokenstore/unsafe_test.go +71 -0

@@ -0,0 +1,71 @@
	1	+	package tokenstore
	2	+
	3	+	import (
	4	+	"context"
	5	+	"fmt"
	6	+	"io"
	7	+	"sync"
	8	+	"testing"
	9	+
	10	+	"github.com/bnema/zerowrap"
	11	+
	12	+	"github.com/bnema/gordon/internal/domain"
	13	+	)
	14	+
	15	+	// newTestUnsafeStore creates a UnsafeStore backed by a temp directory.
	16	+	func newTestUnsafeStore(t testing.T) UnsafeStore {
	17	+	t.Helper()
	18	+	dir := t.TempDir()
	19	+	log := zerowrap.New(zerowrap.Config{Level: "disabled", Output: io.Discard})
	20	+	store, err := NewUnsafeStore(dir, log)
	21	+	if err != nil {
	22	+	t.Fatalf("NewUnsafeStore: %v", err)
	23	+	}
	24	+	return store
	25	+	}
	26	+
	27	+	func TestUnsafeStoreRevokeConcurrentNoLoss(t *testing.T) {
	28	+	// Run with: go test -race ./internal/adapters/out/tokenstore/... -run TestUnsafeStoreRevokeConcurrentNoLoss -v
	29	+	store := newTestUnsafeStore(t)
	30	+
	31	+	ctx := context.Background()
	32	+	const n = 10
	33	+
	34	+	// Save n tokens
	35	+	tokenIDs := make([]string, n)
	36	+	for i := 0; i < n; i++ {
	37	+	id := fmt.Sprintf("token-%d", i)
	38	+	tokenIDs[i] = id
	39	+	tok := &domain.Token{
	40	+	ID: id,
	41	+	Subject: fmt.Sprintf("user-%d", i),
	42	+	Scopes: []string{"admin"},
	43	+	}
	44	+	if err := store.SaveToken(ctx, tok, "jwt-"+id); err != nil {
	45	+	t.Fatal(err)
	46	+	}
	47	+	}
	48	+
	49	+	// Revoke all concurrently
	50	+	var wg sync.WaitGroup
	51	+	for _, id := range tokenIDs {
	52	+	wg.Add(1)
	53	+	go func(id string) {
	54	+	defer wg.Done()
	55	+	_ = store.Revoke(ctx, id)
	56	+	}(id)
	57	+	}
	58	+	wg.Wait()
	59	+
	60	+	// Verify all n token IDs are in the revoked list
	61	+	for _, id := range tokenIDs {
	62	+	revoked, err := store.IsRevoked(ctx, id)
	63	+	if err != nil {
	64	+	t.Errorf("IsRevoked(%q): %v", id, err)
	65	+	continue
	66	+	}
	67	+	if !revoked {
	68	+	t.Errorf("token %q should be revoked but IsRevoked returned false", id)
	69	+	}
	70	+	}
	71	+	}

@@ -0,0 +1,71 @@
	1	+	package tokenstore
	2	+
	3	+	import (
	4	+	"context"
	5	+	"fmt"
	6	+	"io"
	7	+	"sync"
	8	+	"testing"
	9	+
	10	+	"github.com/bnema/zerowrap"
	11	+
	12	+	"github.com/bnema/gordon/internal/domain"
	13	+	)
	14	+
	15	+	// newTestUnsafeStore creates a UnsafeStore backed by a temp directory.
	16	+	func newTestUnsafeStore(t testing.T) UnsafeStore {
	17	+	t.Helper()
	18	+	dir := t.TempDir()
	19	+	log := zerowrap.New(zerowrap.Config{Level: "disabled", Output: io.Discard})
	20	+	store, err := NewUnsafeStore(dir, log)
	21	+	if err != nil {
	22	+	t.Fatalf("NewUnsafeStore: %v", err)
	23	+	}
	24	+	return store
	25	+	}
	26	+
	27	+	func TestUnsafeStoreRevokeConcurrentNoLoss(t *testing.T) {
	28	+	// Run with: go test -race ./internal/adapters/out/tokenstore/... -run TestUnsafeStoreRevokeConcurrentNoLoss -v
	29	+	store := newTestUnsafeStore(t)
	30	+
	31	+	ctx := context.Background()
	32	+	const n = 10
	33	+
	34	+	// Save n tokens
	35	+	tokenIDs := make([]string, n)
	36	+	for i := 0; i < n; i++ {
	37	+	id := fmt.Sprintf("token-%d", i)
	38	+	tokenIDs[i] = id
	39	+	tok := &domain.Token{
	40	+	ID: id,
	41	+	Subject: fmt.Sprintf("user-%d", i),
	42	+	Scopes: []string{"admin"},
	43	+	}
	44	+	if err := store.SaveToken(ctx, tok, "jwt-"+id); err != nil {
	45	+	t.Fatal(err)
	46	+	}
	47	+	}
	48	+
	49	+	// Revoke all concurrently
	50	+	var wg sync.WaitGroup
	51	+	for _, id := range tokenIDs {
	52	+	wg.Add(1)
	53	+	go func(id string) {
	54	+	defer wg.Done()
	55	+	_ = store.Revoke(ctx, id)
	56	+	}(id)
	57	+	}
	58	+	wg.Wait()
	59	+
	60	+	// Verify all n token IDs are in the revoked list
	61	+	for _, id := range tokenIDs {
	62	+	revoked, err := store.IsRevoked(ctx, id)
	63	+	if err != nil {
	64	+	t.Errorf("IsRevoked(%q): %v", id, err)
	65	+	continue
	66	+	}
	67	+	if !revoked {
	68	+	t.Errorf("token %q should be revoked but IsRevoked returned false", id)
	69	+	}
	70	+	}
	71	+	}