Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions internal/api/db_rate_limiter.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,11 @@ func NewDBRateLimiter(pool *pgxpool.Pool) *DBRateLimiter {
// tests with a short-lived context.
func (rl *DBRateLimiter) StartCleanupWorker(ctx context.Context) {
go func() {
defer func() {
if r := recover(); r != nil {
logging.Warnf("db_rate_limiter: StartCleanupWorker goroutine panic: %v", r)
}
}()
ticker := time.NewTicker(dbRateLimiterScheduledCleanupInterval)
defer ticker.Stop()
for {
Expand Down Expand Up @@ -169,6 +174,11 @@ func (rl *DBRateLimiter) maybeCleanup() {
// Run cleanup in background
go func() {
defer rl.cleanupRunning.Store(false)
defer func() {
if r := recover(); r != nil {
logging.Warnf("db_rate_limiter: cleanup goroutine panic: %v", r)
}
}()
rl.cleanup()
}()
}
Expand Down
6 changes: 6 additions & 0 deletions internal/api/handler_accounts.go
Original file line number Diff line number Diff line change
Expand Up @@ -881,6 +881,12 @@ func gcpTokenExchangeAttempt(ctx context.Context, ts oauth2.TokenSource) (Accoun
defer cancel()
tokenChan := make(chan tokenResult, 1)
go func() {
defer func() {
if r := recover(); r != nil {
logging.Warnf("handler_accounts: gcp token-exchange goroutine panic: %v", r)
tokenChan <- tokenResult{err: fmt.Errorf("token exchange panic: %v", r)}
}
}()
tok, err := ts.Token()
tokenChan <- tokenResult{tok: tok, err: err}
}()
Expand Down
12 changes: 9 additions & 3 deletions internal/api/handler_history.go
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,14 @@
h.expireStaleExecutionsSweep(staleExecs)
return
}
go h.expireStaleExecutionsSweep(staleExecs)
go func() {
defer func() {
if r := recover(); r != nil {
logging.Warnf("history: expireStaleExecutionsAsync goroutine panic: %v", r)
}
}()
h.expireStaleExecutionsSweep(staleExecs)
}()
}

// expireStaleExecutionsSweep is the shared sweep body for both branches of
Expand All @@ -237,8 +244,7 @@
// should not abort the best-effort transitions.
func (h *Handler) expireStaleExecutionsSweep(staleExecs []config.PurchaseExecution) {
ctx := context.Background()
for _rvc := range staleExecs {
exec := staleExecs[_rvc]
for _, exec := range staleExecs {

Check failure on line 247 in internal/api/handler_history.go

View workflow job for this annotation

GitHub Actions / Lint Code

rangeValCopy: each iteration copies 304 bytes (consider pointers or indexing) (gocritic)
_, err := h.config.TransitionExecutionStatus(ctx, exec.ExecutionID, []string{"pending", "notified"}, "expired", nil)
if err != nil {
logging.Warnf("history: expire sweep for execution %s failed: %v", exec.ExecutionID, err)
Expand Down
5 changes: 5 additions & 0 deletions internal/auth/service_apikeys.go
Original file line number Diff line number Diff line change
Expand Up @@ -291,6 +291,11 @@ func (s *Service) ValidateUserAPIKey(ctx context.Context, apiKey string) (*UserA
keyID := key.ID
go func() {
if _, sfErr, _ := s.lastUsedSFG.Do(keyID, func() (any, error) {
defer func() {
if r := recover(); r != nil {
logging.Warnf("service_apikeys: UpdateLastUsed goroutine panic: %v", r)
}
}()
updateCtx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
defer cancel()
if err := s.UpdateLastUsed(updateCtx, keyID); err != nil {
Expand Down
52 changes: 52 additions & 0 deletions internal/auth/service_apikeys_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -986,6 +986,58 @@ func TestService_HasAPIKeyPermissionForConstraintsAPI_OwnerCapEnforced(t *testin
})
}

// TestValidateUserAPIKey_UpdateLastUsedPanicIsRecovered proves that a panic
// inside the fire-and-forget UpdateLastUsed goroutine is caught by the
// recover() added in issue #672. Without the recover(), the panic would crash
// the entire test binary (an unrecovered goroutine panic terminates the
// process). The test uses a channel -- closed by the mock Run callback just
// before panicking -- to wait for goroutine execution without time.Sleep.
func TestValidateUserAPIKey_UpdateLastUsedPanicIsRecovered(t *testing.T) {
ctx := context.Background()

apiKey := "test-api-key-panic-123"
hash := sha256.Sum256([]byte(apiKey))
keyHash := base64.RawURLEncoding.EncodeToString(hash[:])

user := &User{
ID: "user-panic",
Email: "panic@example.com",
Active: true,
}
apiKeyRecord := &UserAPIKey{
ID: "key-panic",
UserID: "user-panic",
KeyHash: keyHash,
IsActive: true,
}

mockStore := new(MockStore)
service := &Service{store: mockStore}
t.Cleanup(func() { mockStore.AssertExpectations(t) })

mockStore.On("GetAPIKeyByHash", ctx, keyHash).Return(apiKeyRecord, nil)
mockStore.On("GetUserByID", mock.Anything, "user-panic").Return(user, nil)

// done is closed by the Run callback immediately before panicking so the
// test can wait for goroutine execution without time.Sleep.
done := make(chan struct{})
mockStore.On("UpdateAPIKeyLastUsed", mock.Anything, "key-panic").
Run(func(args mock.Arguments) {
close(done)
panic("injected panic for #672 regression test")
}).
Return(nil)

resultKey, resultUser, err := service.ValidateUserAPIKey(ctx, apiKey)
require.NoError(t, err)
require.Equal(t, apiKeyRecord, resultKey)
require.Equal(t, user, resultUser)

// Block until the goroutine has executed (and panicked + recovered).
// Reaching this line means the process survived -- recover() caught the panic.
<-done
}

func TestService_validateAPIKeyPermissions(t *testing.T) {
ctx := context.Background()

Expand Down
Loading