SECURITY: Sanitize sensitive query parameters in logs (nb-bl3)#4
Merged
SECURITY: Sanitize sensitive query parameters in logs (nb-bl3)#4
Conversation
Added Monitor-based synchronization to prevent multiple threads from simultaneously refreshing the same OAuth token, which was causing authentication failures in multi-threaded environments (Puma, Sidekiq). Implementation: - Added Monitor (reentrant mutex) to HttpClient for thread-safe token refresh - Implemented double-check locking pattern for optimal performance: - Fast path: Check token expiry without locking (most common case) - Slow path: Acquire lock and recheck before refreshing - Only one thread can refresh a token at a time per HttpClient instance Testing: - Added comprehensive concurrency test suite (http_client_concurrency_spec.rb) - Verifies mutex prevents race condition (10 threads → 1 refresh call) - Verifies normal requests remain concurrent when token is fresh - All tests passing Security Impact: - Prevents authentication failures in production multi-threaded apps - Eliminates wasted OAuth refresh requests - Reduces risk of rate limiting due to duplicate refreshes Fixes: nb-6yd 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Added URL query parameter sanitization to prevent OAuth secrets (client_secret, code, refresh_token, access_token) from being logged. This prevents credential leakage through application logs and addresses PCI-DSS, GDPR, and SOC 2 compliance requirements. Implementation: - Added sanitize_url() method to filter sensitive query parameters - Parameters matching sensitive patterns are replaced with [FILTERED] - Added "code" pattern to catch OAuth authorization codes - URL parsing errors are handled gracefully (log original URL) Security Impact BEFORE: - OAuth client secrets logged in plain text - Authorization codes logged (could be replayed) - Refresh tokens exposed in logs - Access tokens visible to log aggregation systems - Compliance violations (PCI-DSS 3.4, GDPR Article 32) Security Impact AFTER: - All sensitive parameters sanitized before logging - Only safe parameters (e.g., grant_type) remain visible - Logs are now safe for aggregation and analysis - Compliance-ready for PCI-DSS, GDPR, SOC 2 Testing: - Added comprehensive URL sanitization test suite - Verifies filtering of client_secret, code, refresh_token, access_token - Verifies non-sensitive parameters remain intact - All 12 logger tests passing Example Output: BEFORE: POST https://api.example.com/oauth/token?client_secret=abc123&code=xyz789 AFTER: POST https://api.example.com/oauth/token?client_secret=%5BFILTERED%5D&code=%5BFILTERED%5D Fixes: nb-bl3 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
1098022 to
2b7efda
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Problem
The logger was logging complete URLs including sensitive OAuth parameters in query strings:
client_secret- OAuth client secretscode- Authorization codes (can be replayed)refresh_token- Long-lived refresh tokensaccess_token- API access tokensThese credentials were being logged in plain text, creating serious security risks:
Solution
Added
sanitize_url()method that:SENSITIVE_PATTERNS[FILTERED]markerEnhanced Sensitive Patterns
Added
/^code$/ipattern to catch OAuth authorization codes:Example Output
Before (INSECURE)
After (SECURE)
Note:
[FILTERED]is URL-encoded as%5BFILTERED%5Dwhen placed in query strings.Testing
New Test Coverage
Added 4 comprehensive tests for URL sanitization:
Test 1: Multiple sensitive parameters
client_secretandcodeare filteredgrant_type) remain visibleTest 2: Refresh token sanitization
refresh_tokenparameter is filteredTest 3: Access token sanitization
access_tokenparameter is filteredTest 4: URLs without parameters
All tests passing: 12 examples, 0 failures
Security Impact
Compliance
Threat Model
Performance Impact
References
🤖 Generated with Claude Code