Skip to content

Commit adbe210

Browse files
Copilotintel352
andcommitted
Fix security issue and linting violations in multiple modules
- Security: Remove authorization and set-cookie headers from httpclient logging to prevent credential leakage - Auth module: Fix all testifylint violations (bool-compare, require-error issues) - Database module: Fix noctx violation by using BeginTx instead of deprecated Begin - Cache module: Fix errcheck, testifylint issues (len, require-error, float-compare) Co-authored-by: intel352 <77607+intel352@users.noreply.github.com>
1 parent 66e1965 commit adbe210

6 files changed

Lines changed: 44 additions & 44 deletions

File tree

modules/auth/module_test.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,7 @@ func TestModule_RegisterConfig(t *testing.T) {
175175
app := NewMockApplication()
176176

177177
err := module.RegisterConfig(app)
178-
assert.NoError(t, err)
178+
require.NoError(t, err)
179179
assert.NotNil(t, module.config)
180180

181181
// Verify config was registered with the app
@@ -202,7 +202,7 @@ func TestModule_Init(t *testing.T) {
202202

203203
app := NewMockApplication()
204204
err := module.Init(app)
205-
assert.NoError(t, err)
205+
require.NoError(t, err)
206206
assert.NotNil(t, module.logger)
207207
}
208208

@@ -218,7 +218,7 @@ func TestModule_Init_InvalidConfig(t *testing.T) {
218218

219219
app := NewMockApplication()
220220
err := module.Init(app)
221-
assert.Error(t, err)
221+
require.Error(t, err)
222222
assert.Contains(t, err.Error(), "configuration validation failed")
223223
}
224224

@@ -231,7 +231,7 @@ func TestModule_StartStop(t *testing.T) {
231231

232232
// Test Start
233233
err := module.Start(ctx)
234-
assert.NoError(t, err)
234+
require.NoError(t, err)
235235

236236
// Test Stop
237237
err = module.Stop(ctx)
@@ -315,7 +315,7 @@ func TestModule_Constructor_InvalidUserStore(t *testing.T) {
315315
}
316316

317317
_, err := constructor(app, services)
318-
assert.Error(t, err)
318+
require.Error(t, err)
319319
assert.Contains(t, err.Error(), "user_store service does not implement UserStore interface")
320320
}
321321

@@ -339,6 +339,6 @@ func TestModule_Constructor_InvalidSessionStore(t *testing.T) {
339339
}
340340

341341
_, err := constructor(app, services)
342-
assert.Error(t, err)
342+
require.Error(t, err)
343343
assert.Contains(t, err.Error(), "session_store service does not implement SessionStore interface")
344344
}

modules/auth/service_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -302,7 +302,7 @@ func TestService_VerifyPassword(t *testing.T) {
302302

303303
// Correct password should verify
304304
err = service.VerifyPassword(hash, password)
305-
assert.NoError(t, err)
305+
require.NoError(t, err)
306306

307307
// Wrong password should fail
308308
err = service.VerifyPassword(hash, "wrongpassword")

modules/auth/stores_test.go

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -26,24 +26,24 @@ func TestMemoryUserStore(t *testing.T) {
2626
// Test CreateUser
2727
err := store.CreateUser(ctx, user)
2828
require.NoError(t, err)
29-
assert.True(t, !user.CreatedAt.IsZero())
30-
assert.True(t, !user.UpdatedAt.IsZero())
29+
assert.False(t, user.CreatedAt.IsZero())
30+
assert.False(t, user.UpdatedAt.IsZero())
3131

3232
// Test duplicate user creation
3333
duplicateUser := &User{
3434
ID: "test-user-123",
3535
Email: "different@example.com",
3636
}
3737
err = store.CreateUser(ctx, duplicateUser)
38-
assert.ErrorIs(t, err, ErrUserAlreadyExists)
38+
require.ErrorIs(t, err, ErrUserAlreadyExists)
3939

4040
// Test duplicate email
4141
duplicateEmailUser := &User{
4242
ID: "different-user",
4343
Email: "test@example.com",
4444
}
4545
err = store.CreateUser(ctx, duplicateEmailUser)
46-
assert.ErrorIs(t, err, ErrUserAlreadyExists)
46+
require.ErrorIs(t, err, ErrUserAlreadyExists)
4747

4848
// Test GetUser
4949
retrievedUser, err := store.GetUser(ctx, user.ID)
@@ -78,19 +78,19 @@ func TestMemoryUserStore(t *testing.T) {
7878
// Test update non-existent user
7979
nonExistentUser := &User{ID: "non-existent"}
8080
err = store.UpdateUser(ctx, nonExistentUser)
81-
assert.ErrorIs(t, err, ErrUserNotFound)
81+
require.ErrorIs(t, err, ErrUserNotFound)
8282

8383
// Test DeleteUser
8484
err = store.DeleteUser(ctx, user.ID)
8585
require.NoError(t, err)
8686

8787
// Verify user is deleted
8888
_, err = store.GetUser(ctx, user.ID)
89-
assert.ErrorIs(t, err, ErrUserNotFound)
89+
require.ErrorIs(t, err, ErrUserNotFound)
9090

9191
// Test delete non-existent user
9292
err = store.DeleteUser(ctx, "non-existent")
93-
assert.ErrorIs(t, err, ErrUserNotFound)
93+
require.ErrorIs(t, err, ErrUserNotFound)
9494

9595
// Test get non-existent user by email
9696
_, err = store.GetUserByEmail(ctx, "nonexistent@example.com")
@@ -141,11 +141,11 @@ func TestMemorySessionStore(t *testing.T) {
141141

142142
// Verify session is deleted
143143
_, err = store.Get(ctx, session.ID)
144-
assert.ErrorIs(t, err, ErrSessionNotFound)
144+
require.ErrorIs(t, err, ErrSessionNotFound)
145145

146146
// Test get non-existent session
147147
_, err = store.Get(ctx, "non-existent")
148-
assert.ErrorIs(t, err, ErrSessionNotFound)
148+
require.ErrorIs(t, err, ErrSessionNotFound)
149149

150150
// Test Cleanup
151151
expiredSession := &Session{
@@ -181,10 +181,10 @@ func TestMemorySessionStore(t *testing.T) {
181181

182182
// Verify cleanup results
183183
_, err = store.Get(ctx, expiredSession.ID)
184-
assert.ErrorIs(t, err, ErrSessionNotFound, "Expired session should be removed")
184+
require.ErrorIs(t, err, ErrSessionNotFound, "Expired session should be removed")
185185

186186
_, err = store.Get(ctx, inactiveSession.ID)
187-
assert.ErrorIs(t, err, ErrSessionNotFound, "Inactive session should be removed")
187+
require.ErrorIs(t, err, ErrSessionNotFound, "Inactive session should be removed")
188188

189189
_, err = store.Get(ctx, validSession.ID)
190190
assert.NoError(t, err, "Valid session should remain")

modules/cache/module_test.go

Lines changed: 23 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ func TestCacheModule(t *testing.T) {
121121

122122
// Test services provided
123123
services := module.(*CacheModule).ProvidesServices()
124-
assert.Equal(t, 1, len(services))
124+
assert.Len(t, services, 1)
125125
assert.Equal(t, ServiceName, services[0].Name)
126126
}
127127

@@ -146,14 +146,14 @@ func TestMemoryCacheOperations(t *testing.T) {
146146

147147
// Test basic operations
148148
err = module.Set(ctx, "test-key", "test-value", time.Minute)
149-
assert.NoError(t, err)
149+
require.NoError(t, err)
150150

151151
value, found := module.Get(ctx, "test-key")
152152
assert.True(t, found)
153153
assert.Equal(t, "test-value", value)
154154

155155
err = module.Delete(ctx, "test-key")
156-
assert.NoError(t, err)
156+
require.NoError(t, err)
157157

158158
_, found = module.Get(ctx, "test-key")
159159
assert.False(t, found)
@@ -166,16 +166,16 @@ func TestMemoryCacheOperations(t *testing.T) {
166166
}
167167

168168
err = module.SetMulti(ctx, items, time.Minute)
169-
assert.NoError(t, err)
169+
require.NoError(t, err)
170170

171171
results, err := module.GetMulti(ctx, []string{"key1", "key2", "key4"})
172-
assert.NoError(t, err)
172+
require.NoError(t, err)
173173
assert.Equal(t, "value1", results["key1"])
174174
assert.Equal(t, "value2", results["key2"])
175175
assert.NotContains(t, results, "key4")
176176

177177
err = module.Flush(ctx)
178-
assert.NoError(t, err)
178+
require.NoError(t, err)
179179

180180
_, found = module.Get(ctx, "key1")
181181
assert.False(t, found)
@@ -211,7 +211,7 @@ func TestExpiration(t *testing.T) {
211211

212212
// Set with short TTL
213213
err = module.Set(ctx, "expires-quickly", "value", time.Second)
214-
assert.NoError(t, err)
214+
require.NoError(t, err)
215215

216216
// Verify it exists
217217
_, found := module.Get(ctx, "expires-quickly")
@@ -300,7 +300,7 @@ func TestRedisOperationsWithMockBehavior(t *testing.T) {
300300

301301
// Test close without connection
302302
err = cache.Close(ctx)
303-
assert.NoError(t, err)
303+
require.NoError(t, err)
304304
}
305305

306306
// TestRedisConfigurationEdgeCases tests edge cases in Redis configuration
@@ -342,16 +342,16 @@ func TestRedisMultiOperationsEmptyInputs(t *testing.T) {
342342

343343
// Test GetMulti with empty keys - should return empty map (no connection needed)
344344
results, err := cache.GetMulti(ctx, []string{})
345-
assert.NoError(t, err)
345+
require.NoError(t, err)
346346
assert.Equal(t, map[string]interface{}{}, results)
347347

348348
// Test SetMulti with empty items - should succeed (no connection needed)
349349
err = cache.SetMulti(ctx, map[string]interface{}{}, time.Minute)
350-
assert.NoError(t, err)
350+
require.NoError(t, err)
351351

352352
// Test DeleteMulti with empty keys - should succeed (no connection needed)
353353
err = cache.DeleteMulti(ctx, []string{})
354-
assert.NoError(t, err)
354+
require.NoError(t, err)
355355
}
356356

357357
// TestRedisConnectWithPassword tests connection configuration with password
@@ -373,11 +373,11 @@ func TestRedisConnectWithPassword(t *testing.T) {
373373
// Test connection with password and different DB - this will fail since no Redis server
374374
// but will exercise the connection configuration code paths
375375
err := cache.Connect(ctx)
376-
assert.Error(t, err) // Expected to fail without Redis server
376+
require.Error(t, err) // Expected to fail without Redis server
377377

378378
// Test Close when client is nil initially
379379
err = cache.Close(ctx)
380-
assert.NoError(t, err)
380+
require.NoError(t, err)
381381
}
382382

383383
// TestRedisJSONMarshaling tests JSON marshaling error scenarios
@@ -445,15 +445,15 @@ func TestRedisFullOperations(t *testing.T) {
445445

446446
// Test Set and Get
447447
err = cache.Set(ctx, "test-key", "test-value", time.Minute)
448-
assert.NoError(t, err)
448+
require.NoError(t, err)
449449

450450
value, found := cache.Get(ctx, "test-key")
451451
assert.True(t, found)
452452
assert.Equal(t, "test-value", value)
453453

454454
// Test Delete
455455
err = cache.Delete(ctx, "test-key")
456-
assert.NoError(t, err)
456+
require.NoError(t, err)
457457

458458
_, found = cache.Get(ctx, "test-key")
459459
assert.False(t, found)
@@ -466,18 +466,18 @@ func TestRedisFullOperations(t *testing.T) {
466466
}
467467

468468
err = cache.SetMulti(ctx, items, time.Minute)
469-
assert.NoError(t, err)
469+
require.NoError(t, err)
470470

471471
results, err := cache.GetMulti(ctx, []string{"key1", "key2", "key3", "nonexistent"})
472-
assert.NoError(t, err)
472+
require.NoError(t, err)
473473
assert.Equal(t, "value1", results["key1"])
474-
assert.Equal(t, float64(42), results["key2"]) // JSON unmarshaling returns numbers as float64
474+
assert.InDelta(t, float64(42), results["key2"], 0.01) // JSON unmarshaling returns numbers as float64
475475
assert.Equal(t, map[string]interface{}{"nested": "value"}, results["key3"])
476476
assert.NotContains(t, results, "nonexistent")
477477

478478
// Test DeleteMulti
479479
err = cache.DeleteMulti(ctx, []string{"key1", "key2"})
480-
assert.NoError(t, err)
480+
require.NoError(t, err)
481481

482482
// Verify deletions
483483
_, found = cache.Get(ctx, "key1")
@@ -490,14 +490,14 @@ func TestRedisFullOperations(t *testing.T) {
490490

491491
// Test Flush
492492
err = cache.Flush(ctx)
493-
assert.NoError(t, err)
493+
require.NoError(t, err)
494494

495495
_, found = cache.Get(ctx, "key3")
496496
assert.False(t, found)
497497

498498
// Test Close
499499
err = cache.Close(ctx)
500-
assert.NoError(t, err)
500+
require.NoError(t, err)
501501
}
502502

503503
// TestRedisGetJSONUnmarshalError tests JSON unmarshaling errors in Get
@@ -526,7 +526,7 @@ func TestRedisGetJSONUnmarshalError(t *testing.T) {
526526
defer cache.Close(ctx)
527527

528528
// Manually insert invalid JSON into Redis
529-
s.Set("invalid-json", "this is not valid JSON {")
529+
_ = s.Set("invalid-json", "this is not valid JSON {")
530530

531531
// Try to get the invalid JSON value
532532
value, found := cache.Get(ctx, "invalid-json")
@@ -567,7 +567,7 @@ func TestRedisGetWithServerError(t *testing.T) {
567567

568568
// Try GetMulti when server is down
569569
results, err := cache.GetMulti(ctx, []string{"key1", "key2"})
570-
assert.Error(t, err)
570+
require.Error(t, err)
571571
assert.Nil(t, results)
572572

573573
// Close cache

modules/database/service.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -260,7 +260,7 @@ func (s *databaseServiceImpl) Begin() (*sql.Tx, error) {
260260
if s.db == nil {
261261
return nil, ErrDatabaseNotConnected
262262
}
263-
tx, err := s.db.Begin()
263+
tx, err := s.db.BeginTx(context.Background(), nil)
264264
if err != nil {
265265
return nil, fmt.Errorf("beginning database transaction: %w", err)
266266
}

modules/httpclient/module.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -730,9 +730,9 @@ func (t *loggingTransport) smartTruncateResponse(dump string, maxSize int) strin
730730
// even when detailed logging is disabled.
731731
func (t *loggingTransport) isImportantHeader(headerName string) bool {
732732
important := []string{
733-
"content-type", "content-length", "authorization", "user-agent",
733+
"content-type", "content-length", "user-agent",
734734
"accept", "cache-control", "x-request-id", "x-correlation-id",
735-
"x-trace-id", "location", "set-cookie",
735+
"x-trace-id", "location",
736736
}
737737

738738
headerLower := strings.ToLower(headerName)

0 commit comments

Comments
 (0)