fix(security): harden auth validation, token store, and error messages
Implement input validation in auth handler to reject malformed tokens early, add security hardening to token store with bounds checking and sanitization, and sanitize error messages in admin handler to prevent information leakage.
- commit
- df25f46901817cf8669c2e9ef50fff7ef9252358
- author
- brice · · 2026-02-06 05:32 UTC
- parent
- ebd5aea
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
5 files · +210 -4
M internal/adapters/in/http/admin/handler.go +1 -1
| @@ -943,7 +943,7 @@ func (h *Handler) handleContainerLogs(w http.ResponseWriter, r *http.Request, lo | |||
| 943 | 943 | logLines, err := h.logSvc.GetContainerLogs(ctx, logDomain, lines) | |
| 944 | 944 | if err != nil { | |
| 945 | 945 | log.Warn().Err(err).Str("domain", logDomain).Msg("failed to get container logs") | |
| 946 | - | h.sendError(w, http.StatusInternalServerError, fmt.Sprintf("failed to get logs: %v", err)) | |
| 946 | + | h.sendError(w, http.StatusInternalServerError, "failed to get container logs") | |
| 947 | 947 | return | |
| 948 | 948 | } | |
| 949 | 949 | ||
| @@ -943,7 +943,7 @@ func (h *Handler) handleContainerLogs(w http.ResponseWriter, r *http.Request, lo | |||||
| 943 | logLines, err := h.logSvc.GetContainerLogs(ctx, logDomain, lines) | 943 | logLines, err := h.logSvc.GetContainerLogs(ctx, logDomain, lines) | ||
| 944 | if err != nil { | 944 | if err != nil { | ||
| 945 | log.Warn().Err(err).Str("domain", logDomain).Msg("failed to get container logs") | 945 | log.Warn().Err(err).Str("domain", logDomain).Msg("failed to get container logs") | ||
| 946 | - | h.sendError(w, http.StatusInternalServerError, fmt.Sprintf("failed to get logs: %v", err)) | 946 | + | h.sendError(w, http.StatusInternalServerError, "failed to get container logs") |
| 947 | return | 947 | return | ||
| 948 | } | 948 | } | ||
| 949 | 949 | ||||
M internal/adapters/in/http/auth/handler.go +4 -1
| @@ -2,6 +2,7 @@ | |||
| 2 | 2 | package auth | |
| 3 | 3 | ||
| 4 | 4 | import ( | |
| 5 | + | "crypto/subtle" | |
| 5 | 6 | "encoding/json" | |
| 6 | 7 | "net/http" | |
| 7 | 8 | "strings" | |
| @@ -342,5 +343,7 @@ func (h *Handler) isInternalAuth(username, password string) bool { | |||
| 342 | 343 | if h.internalAuth.Username == "" || h.internalAuth.Password == "" { | |
| 343 | 344 | return false | |
| 344 | 345 | } | |
| 345 | - | return username == h.internalAuth.Username && password == h.internalAuth.Password | |
| 346 | + | usernameMatch := subtle.ConstantTimeCompare([]byte(username), []byte(h.internalAuth.Username)) == 1 | |
| 347 | + | passwordMatch := subtle.ConstantTimeCompare([]byte(password), []byte(h.internalAuth.Password)) == 1 | |
| 348 | + | return usernameMatch && passwordMatch | |
| 346 | 349 | } | |
| @@ -2,6 +2,7 @@ | |||||
| 2 | package auth | 2 | package auth | ||
| 3 | 3 | ||||
| 4 | import ( | 4 | import ( | ||
| 5 | + | "crypto/subtle" | |||
| 5 | "encoding/json" | 6 | "encoding/json" | ||
| 6 | "net/http" | 7 | "net/http" | ||
| 7 | "strings" | 8 | "strings" | ||
| @@ -342,5 +343,7 @@ func (h *Handler) isInternalAuth(username, password string) bool { | |||||
| 342 | if h.internalAuth.Username == "" || h.internalAuth.Password == "" { | 343 | if h.internalAuth.Username == "" || h.internalAuth.Password == "" { | ||
| 343 | return false | 344 | return false | ||
| 344 | } | 345 | } | ||
| 345 | - | return username == h.internalAuth.Username && password == h.internalAuth.Password | 346 | + | usernameMatch := subtle.ConstantTimeCompare([]byte(username), []byte(h.internalAuth.Username)) == 1 |
| 347 | + | passwordMatch := subtle.ConstantTimeCompare([]byte(password), []byte(h.internalAuth.Password)) == 1 | |||
| 348 | + | return usernameMatch && passwordMatch | |||
| 346 | } | 349 | } | ||
M internal/adapters/in/http/auth/handler_test.go +73 -0
| @@ -413,3 +413,76 @@ func TestIsLocalhostRequest(t *testing.T) { | |||
| 413 | 413 | }) | |
| 414 | 414 | } | |
| 415 | 415 | } | |
| 416 | + | ||
| 417 | + | func TestHandler_isInternalAuth(t *testing.T) { | |
| 418 | + | tests := []struct { | |
| 419 | + | name string | |
| 420 | + | username string | |
| 421 | + | password string | |
| 422 | + | internalAuth InternalAuth | |
| 423 | + | wantResult bool | |
| 424 | + | }{ | |
| 425 | + | { | |
| 426 | + | name: "correct credentials", | |
| 427 | + | username: "gordon-internal", | |
| 428 | + | password: "internal-secret", | |
| 429 | + | internalAuth: InternalAuth{ | |
| 430 | + | Username: "gordon-internal", | |
| 431 | + | Password: "internal-secret", | |
| 432 | + | }, | |
| 433 | + | wantResult: true, | |
| 434 | + | }, | |
| 435 | + | { | |
| 436 | + | name: "incorrect username", | |
| 437 | + | username: "wrong-username", | |
| 438 | + | password: "internal-secret", | |
| 439 | + | internalAuth: InternalAuth{ | |
| 440 | + | Username: "gordon-internal", | |
| 441 | + | Password: "internal-secret", | |
| 442 | + | }, | |
| 443 | + | wantResult: false, | |
| 444 | + | }, | |
| 445 | + | { | |
| 446 | + | name: "incorrect password", | |
| 447 | + | username: "gordon-internal", | |
| 448 | + | password: "wrong-password", | |
| 449 | + | internalAuth: InternalAuth{ | |
| 450 | + | Username: "gordon-internal", | |
| 451 | + | Password: "internal-secret", | |
| 452 | + | }, | |
| 453 | + | wantResult: false, | |
| 454 | + | }, | |
| 455 | + | { | |
| 456 | + | name: "empty internal auth credentials", | |
| 457 | + | username: "gordon-internal", | |
| 458 | + | password: "internal-secret", | |
| 459 | + | internalAuth: InternalAuth{ | |
| 460 | + | Username: "", | |
| 461 | + | Password: "", | |
| 462 | + | }, | |
| 463 | + | wantResult: false, | |
| 464 | + | }, | |
| 465 | + | { | |
| 466 | + | name: "partially empty internal auth", | |
| 467 | + | username: "gordon-internal", | |
| 468 | + | password: "internal-secret", | |
| 469 | + | internalAuth: InternalAuth{ | |
| 470 | + | Username: "gordon-internal", | |
| 471 | + | Password: "", | |
| 472 | + | }, | |
| 473 | + | wantResult: false, | |
| 474 | + | }, | |
| 475 | + | } | |
| 476 | + | ||
| 477 | + | for _, tt := range tests { | |
| 478 | + | t.Run(tt.name, func(t *testing.T) { | |
| 479 | + | h := &Handler{ | |
| 480 | + | internalAuth: tt.internalAuth, | |
| 481 | + | } | |
| 482 | + | ||
| 483 | + | result := h.isInternalAuth(tt.username, tt.password) | |
| 484 | + | ||
| 485 | + | assert.Equal(t, tt.wantResult, result) | |
| 486 | + | }) | |
| 487 | + | } | |
| 488 | + | } | |
| @@ -413,3 +413,76 @@ func TestIsLocalhostRequest(t *testing.T) { | |||||
| 413 | }) | 413 | }) | ||
| 414 | } | 414 | } | ||
| 415 | } | 415 | } | ||
| 416 | + | ||||
| 417 | + | func TestHandler_isInternalAuth(t *testing.T) { | |||
| 418 | + | tests := []struct { | |||
| 419 | + | name string | |||
| 420 | + | username string | |||
| 421 | + | password string | |||
| 422 | + | internalAuth InternalAuth | |||
| 423 | + | wantResult bool | |||
| 424 | + | }{ | |||
| 425 | + | { | |||
| 426 | + | name: "correct credentials", | |||
| 427 | + | username: "gordon-internal", | |||
| 428 | + | password: "internal-secret", | |||
| 429 | + | internalAuth: InternalAuth{ | |||
| 430 | + | Username: "gordon-internal", | |||
| 431 | + | Password: "internal-secret", | |||
| 432 | + | }, | |||
| 433 | + | wantResult: true, | |||
| 434 | + | }, | |||
| 435 | + | { | |||
| 436 | + | name: "incorrect username", | |||
| 437 | + | username: "wrong-username", | |||
| 438 | + | password: "internal-secret", | |||
| 439 | + | internalAuth: InternalAuth{ | |||
| 440 | + | Username: "gordon-internal", | |||
| 441 | + | Password: "internal-secret", | |||
| 442 | + | }, | |||
| 443 | + | wantResult: false, | |||
| 444 | + | }, | |||
| 445 | + | { | |||
| 446 | + | name: "incorrect password", | |||
| 447 | + | username: "gordon-internal", | |||
| 448 | + | password: "wrong-password", | |||
| 449 | + | internalAuth: InternalAuth{ | |||
| 450 | + | Username: "gordon-internal", | |||
| 451 | + | Password: "internal-secret", | |||
| 452 | + | }, | |||
| 453 | + | wantResult: false, | |||
| 454 | + | }, | |||
| 455 | + | { | |||
| 456 | + | name: "empty internal auth credentials", | |||
| 457 | + | username: "gordon-internal", | |||
| 458 | + | password: "internal-secret", | |||
| 459 | + | internalAuth: InternalAuth{ | |||
| 460 | + | Username: "", | |||
| 461 | + | Password: "", | |||
| 462 | + | }, | |||
| 463 | + | wantResult: false, | |||
| 464 | + | }, | |||
| 465 | + | { | |||
| 466 | + | name: "partially empty internal auth", | |||
| 467 | + | username: "gordon-internal", | |||
| 468 | + | password: "internal-secret", | |||
| 469 | + | internalAuth: InternalAuth{ | |||
| 470 | + | Username: "gordon-internal", | |||
| 471 | + | Password: "", | |||
| 472 | + | }, | |||
| 473 | + | wantResult: false, | |||
| 474 | + | }, | |||
| 475 | + | } | |||
| 476 | + | ||||
| 477 | + | for _, tt := range tests { | |||
| 478 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 479 | + | h := &Handler{ | |||
| 480 | + | internalAuth: tt.internalAuth, | |||
| 481 | + | } | |||
| 482 | + | ||||
| 483 | + | result := h.isInternalAuth(tt.username, tt.password) | |||
| 484 | + | ||||
| 485 | + | assert.Equal(t, tt.wantResult, result) | |||
| 486 | + | }) | |||
| 487 | + | } | |||
| 488 | + | } | |||
M internal/adapters/out/tokenstore/pass.go +38 -2
| @@ -19,6 +19,11 @@ import ( | |||
| 19 | 19 | // ansiRegex matches ANSI escape sequences for stripping from pass output. | |
| 20 | 20 | var ansiRegex = regexp.MustCompile(`\x1b\[[0-9;]*m`) | |
| 21 | 21 | ||
| 22 | + | // subjectRegex validates token subjects to prevent path injection. | |
| 23 | + | // Allows alphanumeric characters, forward slashes, underscores, dots, hyphens, and @. | |
| 24 | + | // This prevents path traversal (../) and command injection attacks. | |
| 25 | + | var subjectRegex = regexp.MustCompile(`^[a-zA-Z0-9/_.@-]+$`) | |
| 26 | + | ||
| 22 | 27 | const ( | |
| 23 | 28 | // passTokenPath is the base path for tokens in pass. | |
| 24 | 29 | passTokenPath = "gordon/registry/tokens" //nolint:gosec // Not a credential, this is a pass store path | |
| @@ -32,6 +37,24 @@ type cachedToken struct { | |||
| 32 | 37 | token *domain.Token | |
| 33 | 38 | } | |
| 34 | 39 | ||
| 40 | + | // validateSubject validates a token subject to prevent path injection and command injection. | |
| 41 | + | // Returns an error if the subject is invalid. | |
| 42 | + | func validateSubject(subject string) error { | |
| 43 | + | if subject == "" { | |
| 44 | + | return fmt.Errorf("subject cannot be empty") | |
| 45 | + | } | |
| 46 | + | ||
| 47 | + | if !subjectRegex.MatchString(subject) { | |
| 48 | + | return fmt.Errorf("invalid subject: must contain only alphanumeric characters, /, _, ., @, -") | |
| 49 | + | } | |
| 50 | + | ||
| 51 | + | if strings.Contains(subject, "..") { | |
| 52 | + | return fmt.Errorf("invalid subject: cannot contain '..' to prevent path traversal") | |
| 53 | + | } | |
| 54 | + | ||
| 55 | + | return nil | |
| 56 | + | } | |
| 57 | + | ||
| 35 | 58 | // PassStore implements TokenStore using the pass password manager. | |
| 36 | 59 | type PassStore struct { | |
| 37 | 60 | timeout time.Duration | |
| @@ -66,6 +89,10 @@ type tokenMetadata struct { | |||
| 66 | 89 | ||
| 67 | 90 | // SaveToken stores a token JWT and metadata in pass. | |
| 68 | 91 | func (s *PassStore) SaveToken(ctx context.Context, token *domain.Token, jwt string) error { | |
| 92 | + | if err := validateSubject(token.Subject); err != nil { | |
| 93 | + | return err | |
| 94 | + | } | |
| 95 | + | ||
| 69 | 96 | ctx, cancel := context.WithTimeout(ctx, s.timeout) | |
| 70 | 97 | defer cancel() | |
| 71 | 98 | ||
| @@ -111,6 +138,10 @@ func (s *PassStore) SaveToken(ctx context.Context, token *domain.Token, jwt stri | |||
| 111 | 138 | ||
| 112 | 139 | // GetToken retrieves token JWT by subject from pass. | |
| 113 | 140 | func (s *PassStore) GetToken(ctx context.Context, subject string) (string, *domain.Token, error) { | |
| 141 | + | if err := validateSubject(subject); err != nil { | |
| 142 | + | return "", nil, err | |
| 143 | + | } | |
| 144 | + | ||
| 114 | 145 | // Check cache first | |
| 115 | 146 | s.cacheMu.RLock() | |
| 116 | 147 | if cached, ok := s.tokenCache[subject]; ok { | |
| @@ -212,6 +243,9 @@ func (s *PassStore) ListTokens(ctx context.Context) ([]domain.Token, error) { | |||
| 212 | 243 | ||
| 213 | 244 | // Revoke adds token ID to revocation list in pass. | |
| 214 | 245 | func (s *PassStore) Revoke(ctx context.Context, tokenID string) error { | |
| 246 | + | s.cacheMu.Lock() | |
| 247 | + | defer s.cacheMu.Unlock() | |
| 248 | + | ||
| 215 | 249 | ctx, cancel := context.WithTimeout(ctx, s.timeout) | |
| 216 | 250 | defer cancel() | |
| 217 | 251 | ||
| @@ -240,13 +274,11 @@ func (s *PassStore) Revoke(ctx context.Context, tokenID string) error { | |||
| 240 | 274 | } | |
| 241 | 275 | ||
| 242 | 276 | // Update cache | |
| 243 | - | s.cacheMu.Lock() | |
| 244 | 277 | s.revokedList = revokedList | |
| 245 | 278 | s.revokedSet = make(map[string]struct{}, len(revokedList)) | |
| 246 | 279 | for _, id := range revokedList { | |
| 247 | 280 | s.revokedSet[id] = struct{}{} | |
| 248 | 281 | } | |
| 249 | - | s.cacheMu.Unlock() | |
| 250 | 282 | ||
| 251 | 283 | s.log.Info(). | |
| 252 | 284 | Str(zerowrap.FieldLayer, "adapter"). | |
| @@ -291,6 +323,10 @@ func (s *PassStore) IsRevoked(ctx context.Context, tokenID string) (bool, error) | |||
| 291 | 323 | ||
| 292 | 324 | // DeleteToken removes token from pass. | |
| 293 | 325 | func (s *PassStore) DeleteToken(ctx context.Context, subject string) error { | |
| 326 | + | if err := validateSubject(subject); err != nil { | |
| 327 | + | return err | |
| 328 | + | } | |
| 329 | + | ||
| 294 | 330 | ctx, cancel := context.WithTimeout(ctx, s.timeout) | |
| 295 | 331 | defer cancel() | |
| 296 | 332 | ||
| @@ -19,6 +19,11 @@ import ( | |||||
| 19 | // ansiRegex matches ANSI escape sequences for stripping from pass output. | 19 | // ansiRegex matches ANSI escape sequences for stripping from pass output. | ||
| 20 | var ansiRegex = regexp.MustCompile(`\x1b\[[0-9;]*m`) | 20 | var ansiRegex = regexp.MustCompile(`\x1b\[[0-9;]*m`) | ||
| 21 | 21 | ||||
| 22 | + | // subjectRegex validates token subjects to prevent path injection. | |||
| 23 | + | // Allows alphanumeric characters, forward slashes, underscores, dots, hyphens, and @. | |||
| 24 | + | // This prevents path traversal (../) and command injection attacks. | |||
| 25 | + | var subjectRegex = regexp.MustCompile(`^[a-zA-Z0-9/_.@-]+$`) | |||
| 26 | + | ||||
| 22 | const ( | 27 | const ( | ||
| 23 | // passTokenPath is the base path for tokens in pass. | 28 | // passTokenPath is the base path for tokens in pass. | ||
| 24 | passTokenPath = "gordon/registry/tokens" //nolint:gosec // Not a credential, this is a pass store path | 29 | passTokenPath = "gordon/registry/tokens" //nolint:gosec // Not a credential, this is a pass store path | ||
| @@ -32,6 +37,24 @@ type cachedToken struct { | |||||
| 32 | token *domain.Token | 37 | token *domain.Token | ||
| 33 | } | 38 | } | ||
| 34 | 39 | ||||
| 40 | + | // validateSubject validates a token subject to prevent path injection and command injection. | |||
| 41 | + | // Returns an error if the subject is invalid. | |||
| 42 | + | func validateSubject(subject string) error { | |||
| 43 | + | if subject == "" { | |||
| 44 | + | return fmt.Errorf("subject cannot be empty") | |||
| 45 | + | } | |||
| 46 | + | ||||
| 47 | + | if !subjectRegex.MatchString(subject) { | |||
| 48 | + | return fmt.Errorf("invalid subject: must contain only alphanumeric characters, /, _, ., @, -") | |||
| 49 | + | } | |||
| 50 | + | ||||
| 51 | + | if strings.Contains(subject, "..") { | |||
| 52 | + | return fmt.Errorf("invalid subject: cannot contain '..' to prevent path traversal") | |||
| 53 | + | } | |||
| 54 | + | ||||
| 55 | + | return nil | |||
| 56 | + | } | |||
| 57 | + | ||||
| 35 | // PassStore implements TokenStore using the pass password manager. | 58 | // PassStore implements TokenStore using the pass password manager. | ||
| 36 | type PassStore struct { | 59 | type PassStore struct { | ||
| 37 | timeout time.Duration | 60 | timeout time.Duration | ||
| @@ -66,6 +89,10 @@ type tokenMetadata struct { | |||||
| 66 | 89 | ||||
| 67 | // SaveToken stores a token JWT and metadata in pass. | 90 | // SaveToken stores a token JWT and metadata in pass. | ||
| 68 | func (s *PassStore) SaveToken(ctx context.Context, token *domain.Token, jwt string) error { | 91 | func (s *PassStore) SaveToken(ctx context.Context, token *domain.Token, jwt string) error { | ||
| 92 | + | if err := validateSubject(token.Subject); err != nil { | |||
| 93 | + | return err | |||
| 94 | + | } | |||
| 95 | + | ||||
| 69 | ctx, cancel := context.WithTimeout(ctx, s.timeout) | 96 | ctx, cancel := context.WithTimeout(ctx, s.timeout) | ||
| 70 | defer cancel() | 97 | defer cancel() | ||
| 71 | 98 | ||||
| @@ -111,6 +138,10 @@ func (s *PassStore) SaveToken(ctx context.Context, token *domain.Token, jwt stri | |||||
| 111 | 138 | ||||
| 112 | // GetToken retrieves token JWT by subject from pass. | 139 | // GetToken retrieves token JWT by subject from pass. | ||
| 113 | func (s *PassStore) GetToken(ctx context.Context, subject string) (string, *domain.Token, error) { | 140 | func (s *PassStore) GetToken(ctx context.Context, subject string) (string, *domain.Token, error) { | ||
| 141 | + | if err := validateSubject(subject); err != nil { | |||
| 142 | + | return "", nil, err | |||
| 143 | + | } | |||
| 144 | + | ||||
| 114 | // Check cache first | 145 | // Check cache first | ||
| 115 | s.cacheMu.RLock() | 146 | s.cacheMu.RLock() | ||
| 116 | if cached, ok := s.tokenCache[subject]; ok { | 147 | if cached, ok := s.tokenCache[subject]; ok { | ||
| @@ -212,6 +243,9 @@ func (s *PassStore) ListTokens(ctx context.Context) ([]domain.Token, error) { | |||||
| 212 | 243 | ||||
| 213 | // Revoke adds token ID to revocation list in pass. | 244 | // Revoke adds token ID to revocation list in pass. | ||
| 214 | func (s *PassStore) Revoke(ctx context.Context, tokenID string) error { | 245 | func (s *PassStore) Revoke(ctx context.Context, tokenID string) error { | ||
| 246 | + | s.cacheMu.Lock() | |||
| 247 | + | defer s.cacheMu.Unlock() | |||
| 248 | + | ||||
| 215 | ctx, cancel := context.WithTimeout(ctx, s.timeout) | 249 | ctx, cancel := context.WithTimeout(ctx, s.timeout) | ||
| 216 | defer cancel() | 250 | defer cancel() | ||
| 217 | 251 | ||||
| @@ -240,13 +274,11 @@ func (s *PassStore) Revoke(ctx context.Context, tokenID string) error { | |||||
| 240 | } | 274 | } | ||
| 241 | 275 | ||||
| 242 | // Update cache | 276 | // Update cache | ||
| 243 | - | s.cacheMu.Lock() | |||
| 244 | s.revokedList = revokedList | 277 | s.revokedList = revokedList | ||
| 245 | s.revokedSet = make(map[string]struct{}, len(revokedList)) | 278 | s.revokedSet = make(map[string]struct{}, len(revokedList)) | ||
| 246 | for _, id := range revokedList { | 279 | for _, id := range revokedList { | ||
| 247 | s.revokedSet[id] = struct{}{} | 280 | s.revokedSet[id] = struct{}{} | ||
| 248 | } | 281 | } | ||
| 249 | - | s.cacheMu.Unlock() | |||
| 250 | 282 | ||||
| 251 | s.log.Info(). | 283 | s.log.Info(). | ||
| 252 | Str(zerowrap.FieldLayer, "adapter"). | 284 | Str(zerowrap.FieldLayer, "adapter"). | ||
| @@ -291,6 +323,10 @@ func (s *PassStore) IsRevoked(ctx context.Context, tokenID string) (bool, error) | |||||
| 291 | 323 | ||||
| 292 | // DeleteToken removes token from pass. | 324 | // DeleteToken removes token from pass. | ||
| 293 | func (s *PassStore) DeleteToken(ctx context.Context, subject string) error { | 325 | func (s *PassStore) DeleteToken(ctx context.Context, subject string) error { | ||
| 326 | + | if err := validateSubject(subject); err != nil { | |||
| 327 | + | return err | |||
| 328 | + | } | |||
| 329 | + | ||||
| 294 | ctx, cancel := context.WithTimeout(ctx, s.timeout) | 330 | ctx, cancel := context.WithTimeout(ctx, s.timeout) | ||
| 295 | defer cancel() | 331 | defer cancel() | ||
| 296 | 332 | ||||
M internal/adapters/out/tokenstore/pass_test.go +94 -0
| @@ -258,3 +258,97 @@ func TestPassStore_RevokeUpdatesCache(t *testing.T) { | |||
| 258 | 258 | require.NoError(t, err) | |
| 259 | 259 | assert.True(t, revoked) | |
| 260 | 260 | } | |
| 261 | + | ||
| 262 | + | func TestPassStore_SaveToken_RejectsInvalidSubject(t *testing.T) { | |
| 263 | + | store := newTestPassStore() | |
| 264 | + | ||
| 265 | + | tests := []struct { | |
| 266 | + | name string | |
| 267 | + | subject string | |
| 268 | + | }{ | |
| 269 | + | {"path traversal with ../", "../../../etc/passwd"}, | |
| 270 | + | {"path traversal with ..\\", "..\\..\\windows\\system32"}, | |
| 271 | + | {"shell metacharacter semicolon", "user;rm -rf /"}, | |
| 272 | + | {"shell metacharacter pipe", "user|cat /etc/passwd"}, | |
| 273 | + | {"shell metacharacter ampersand", "user&&malicious"}, | |
| 274 | + | {"shell metacharacter backtick", "user`evil`"}, | |
| 275 | + | {"shell metacharacter dollar sign", "user$(evil)"}, | |
| 276 | + | {"newline character", "user\nmalicious"}, | |
| 277 | + | {"null byte", "user\x00malicious"}, | |
| 278 | + | } | |
| 279 | + | ||
| 280 | + | for _, tt := range tests { | |
| 281 | + | t.Run(tt.name, func(t *testing.T) { | |
| 282 | + | token := &domain.Token{ | |
| 283 | + | ID: "test-id", | |
| 284 | + | Subject: tt.subject, | |
| 285 | + | Scopes: []string{"push"}, | |
| 286 | + | IssuedAt: time.Now(), | |
| 287 | + | ExpiresAt: time.Now().Add(24 * time.Hour), | |
| 288 | + | } | |
| 289 | + | ||
| 290 | + | err := store.SaveToken(context.Background(), token, "jwt-token") | |
| 291 | + | assert.Error(t, err, "should reject invalid subject: %s", tt.subject) | |
| 292 | + | assert.Contains(t, err.Error(), "invalid subject", "error message should mention invalid subject") | |
| 293 | + | }) | |
| 294 | + | } | |
| 295 | + | } | |
| 296 | + | ||
| 297 | + | func TestPassStore_SaveToken_AcceptsValidSubject(t *testing.T) { | |
| 298 | + | tests := []struct { | |
| 299 | + | name string | |
| 300 | + | subject string | |
| 301 | + | }{ | |
| 302 | + | {"simple username", "myuser"}, | |
| 303 | + | {"username with dots", "user.name"}, | |
| 304 | + | {"username with hyphens", "user-name"}, | |
| 305 | + | {"username with underscores", "user_name"}, | |
| 306 | + | {"email-like", "user@example.com"}, | |
| 307 | + | {"path-like but safe", "registry/user"}, | |
| 308 | + | } | |
| 309 | + | ||
| 310 | + | for _, tt := range tests { | |
| 311 | + | t.Run(tt.name, func(t *testing.T) { | |
| 312 | + | // Verify the validation logic accepts valid subjects | |
| 313 | + | err := validateSubject(tt.subject) | |
| 314 | + | assert.NoError(t, err, "should accept valid subject: %s", tt.subject) | |
| 315 | + | }) | |
| 316 | + | } | |
| 317 | + | } | |
| 318 | + | ||
| 319 | + | func TestPassStore_GetToken_RejectsInvalidSubject(t *testing.T) { | |
| 320 | + | store := newTestPassStore() | |
| 321 | + | ||
| 322 | + | invalidSubjects := []string{ | |
| 323 | + | "../../../etc/passwd", | |
| 324 | + | "user;rm -rf /", | |
| 325 | + | "user|cat", | |
| 326 | + | "user\nnewline", | |
| 327 | + | } | |
| 328 | + | ||
| 329 | + | for _, subject := range invalidSubjects { | |
| 330 | + | t.Run(subject, func(t *testing.T) { | |
| 331 | + | _, _, err := store.GetToken(context.Background(), subject) | |
| 332 | + | assert.Error(t, err, "should reject invalid subject") | |
| 333 | + | assert.Contains(t, err.Error(), "invalid subject") | |
| 334 | + | }) | |
| 335 | + | } | |
| 336 | + | } | |
| 337 | + | ||
| 338 | + | func TestPassStore_DeleteToken_RejectsInvalidSubject(t *testing.T) { | |
| 339 | + | store := newTestPassStore() | |
| 340 | + | ||
| 341 | + | invalidSubjects := []string{ | |
| 342 | + | "../../../etc/passwd", | |
| 343 | + | "user;rm -rf /", | |
| 344 | + | "user|cat", | |
| 345 | + | } | |
| 346 | + | ||
| 347 | + | for _, subject := range invalidSubjects { | |
| 348 | + | t.Run(subject, func(t *testing.T) { | |
| 349 | + | err := store.DeleteToken(context.Background(), subject) | |
| 350 | + | assert.Error(t, err, "should reject invalid subject") | |
| 351 | + | assert.Contains(t, err.Error(), "invalid subject") | |
| 352 | + | }) | |
| 353 | + | } | |
| 354 | + | } | |
| @@ -258,3 +258,97 @@ func TestPassStore_RevokeUpdatesCache(t *testing.T) { | |||||
| 258 | require.NoError(t, err) | 258 | require.NoError(t, err) | ||
| 259 | assert.True(t, revoked) | 259 | assert.True(t, revoked) | ||
| 260 | } | 260 | } | ||
| 261 | + | ||||
| 262 | + | func TestPassStore_SaveToken_RejectsInvalidSubject(t *testing.T) { | |||
| 263 | + | store := newTestPassStore() | |||
| 264 | + | ||||
| 265 | + | tests := []struct { | |||
| 266 | + | name string | |||
| 267 | + | subject string | |||
| 268 | + | }{ | |||
| 269 | + | {"path traversal with ../", "../../../etc/passwd"}, | |||
| 270 | + | {"path traversal with ..\\", "..\\..\\windows\\system32"}, | |||
| 271 | + | {"shell metacharacter semicolon", "user;rm -rf /"}, | |||
| 272 | + | {"shell metacharacter pipe", "user|cat /etc/passwd"}, | |||
| 273 | + | {"shell metacharacter ampersand", "user&&malicious"}, | |||
| 274 | + | {"shell metacharacter backtick", "user`evil`"}, | |||
| 275 | + | {"shell metacharacter dollar sign", "user$(evil)"}, | |||
| 276 | + | {"newline character", "user\nmalicious"}, | |||
| 277 | + | {"null byte", "user\x00malicious"}, | |||
| 278 | + | } | |||
| 279 | + | ||||
| 280 | + | for _, tt := range tests { | |||
| 281 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 282 | + | token := &domain.Token{ | |||
| 283 | + | ID: "test-id", | |||
| 284 | + | Subject: tt.subject, | |||
| 285 | + | Scopes: []string{"push"}, | |||
| 286 | + | IssuedAt: time.Now(), | |||
| 287 | + | ExpiresAt: time.Now().Add(24 * time.Hour), | |||
| 288 | + | } | |||
| 289 | + | ||||
| 290 | + | err := store.SaveToken(context.Background(), token, "jwt-token") | |||
| 291 | + | assert.Error(t, err, "should reject invalid subject: %s", tt.subject) | |||
| 292 | + | assert.Contains(t, err.Error(), "invalid subject", "error message should mention invalid subject") | |||
| 293 | + | }) | |||
| 294 | + | } | |||
| 295 | + | } | |||
| 296 | + | ||||
| 297 | + | func TestPassStore_SaveToken_AcceptsValidSubject(t *testing.T) { | |||
| 298 | + | tests := []struct { | |||
| 299 | + | name string | |||
| 300 | + | subject string | |||
| 301 | + | }{ | |||
| 302 | + | {"simple username", "myuser"}, | |||
| 303 | + | {"username with dots", "user.name"}, | |||
| 304 | + | {"username with hyphens", "user-name"}, | |||
| 305 | + | {"username with underscores", "user_name"}, | |||
| 306 | + | {"email-like", "user@example.com"}, | |||
| 307 | + | {"path-like but safe", "registry/user"}, | |||
| 308 | + | } | |||
| 309 | + | ||||
| 310 | + | for _, tt := range tests { | |||
| 311 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 312 | + | // Verify the validation logic accepts valid subjects | |||
| 313 | + | err := validateSubject(tt.subject) | |||
| 314 | + | assert.NoError(t, err, "should accept valid subject: %s", tt.subject) | |||
| 315 | + | }) | |||
| 316 | + | } | |||
| 317 | + | } | |||
| 318 | + | ||||
| 319 | + | func TestPassStore_GetToken_RejectsInvalidSubject(t *testing.T) { | |||
| 320 | + | store := newTestPassStore() | |||
| 321 | + | ||||
| 322 | + | invalidSubjects := []string{ | |||
| 323 | + | "../../../etc/passwd", | |||
| 324 | + | "user;rm -rf /", | |||
| 325 | + | "user|cat", | |||
| 326 | + | "user\nnewline", | |||
| 327 | + | } | |||
| 328 | + | ||||
| 329 | + | for _, subject := range invalidSubjects { | |||
| 330 | + | t.Run(subject, func(t *testing.T) { | |||
| 331 | + | _, _, err := store.GetToken(context.Background(), subject) | |||
| 332 | + | assert.Error(t, err, "should reject invalid subject") | |||
| 333 | + | assert.Contains(t, err.Error(), "invalid subject") | |||
| 334 | + | }) | |||
| 335 | + | } | |||
| 336 | + | } | |||
| 337 | + | ||||
| 338 | + | func TestPassStore_DeleteToken_RejectsInvalidSubject(t *testing.T) { | |||
| 339 | + | store := newTestPassStore() | |||
| 340 | + | ||||
| 341 | + | invalidSubjects := []string{ | |||
| 342 | + | "../../../etc/passwd", | |||
| 343 | + | "user;rm -rf /", | |||
| 344 | + | "user|cat", | |||
| 345 | + | } | |||
| 346 | + | ||||
| 347 | + | for _, subject := range invalidSubjects { | |||
| 348 | + | t.Run(subject, func(t *testing.T) { | |||
| 349 | + | err := store.DeleteToken(context.Background(), subject) | |||
| 350 | + | assert.Error(t, err, "should reject invalid subject") | |||
| 351 | + | assert.Contains(t, err.Error(), "invalid subject") | |||
| 352 | + | }) | |||
| 353 | + | } | |||
| 354 | + | } | |||