🔒 LastFM: Fix information leak in debug logs

[segin]

🎯 What: The vulnerability fixed is an information leak in logs where the password length and session keys were being recorded.
⚠️ Risk: Logging the length of a password reduces the search space for an attacker. Logging session keys allows unauthorized access to the user's Last.fm account.
🛡️ Solution: Removed these sensitive details from the DEBUG_LOG_LAZY calls in src/lastfm/LastFM.cpp.

---

PR created automatically by Jules for task 17259440130858798210 started by @segin

[google-labs-jules]

👋 Jules, reporting for duty! I'm here to lend a hand with this pull request.

When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down.

I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job!

For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.

---

For security, I will only act on instructions from the user who triggered this task.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

[github-actions]

Claude encountered an error —— View job

---

Pull Request Review

Todo List:

• Examine the main security fix in src/lastfm/LastFM.cpp
• Review GitHub workflow changes
• Assess security completeness and potential issues
• Check for test coverage considerations
• Provide comprehensive feedback on code quality and best practices

---

Security Fix Review ✅

🔒 Main Security Changes - src/lastfm/LastFM.cpp

Good fixes implemented:

1. Line 73: Removed password length logging: DEBUG_LOG_LAZY("lastfm", "Password loaded (", m_password.length(), " characters)") → DEBUG_LOG_LAZY("lastfm", "Password loaded")
2. Line 76: Removed session key prefix logging: DEBUG_LOG_LAZY("lastfm", "Session key loaded: ", m_session_key.substr(0, 8), "...") → DEBUG_LOG_LAZY("lastfm", "Session key loaded")
3. Line 200: Removed full session key logging from handshake: DEBUG_LOG_LAZY("lastfm", "Handshake successful. Session Key: ", m_session_key) → DEBUG_LOG_LAZY("lastfm", "Handshake successful")

✅ Security Assessment:

• Password length leak: Correctly fixed - knowing password length significantly reduces brute-force search space
• Session key leak: Correctly fixed - session keys provide full account access and should never be logged
• Fix completeness: Appears comprehensive - I verified no other session key or password length logging remains in the file

Code Quality Assessment

✅ Strengths:

1. Minimal, targeted changes - Only removed sensitive data without affecting functionality
2. Consistent logging style - Maintains informative but safe logging patterns
3. No functional impact - Changes are purely cosmetic security improvements

⚠️ Security Considerations:

1. Config file storage: Password is still stored in plaintext in lastfm.conf (lines 107, 72) - consider warning users about file permissions
2. Memory exposure: Raw password string exists in memory - consider using secure string classes for sensitive data
3. URL logging: Now-playing and submission URLs are still logged (lines 79, 82, 201, 202) - these contain API endpoints but are generally safe

🔧 Code Quality Observations:

1. Threading safety: Code properly follows the documented Public/Private Lock Pattern
2. Error handling: Good exception-based error handling throughout
3. Performance: Efficient string operations with pre-allocation as noted in comments

GitHub Workflow Updates ✅

Appropriate dependency updates:

• actions/cache@v3 → v4
• actions/upload-artifact@v3 → v4
• actions/setup-python@v4 → v5
• Added missing github_token parameter to Claude actions
• Fixed permissions: pull-requests: read → write, issues: read → write

These are standard maintenance updates and security improvements.

Test Coverage Analysis ✅

Excellent test coverage exists:

• test_lastfm_lazy_logging_properties.cpp - Tests lazy evaluation of debug logging (directly relevant to this fix)
• test_lastfm_config_roundtrip_properties.cpp - Tests config loading/saving
• test_lastfm_thread_safety_properties.cpp - Tests threading behavior
• Multiple other LastFM property-based tests using RapidCheck framework

✅ Testing Assessment:
The existing test suite should validate that the logging changes don't break functionality, and the lazy logging tests specifically ensure debug statements work correctly.

Recommendations

🚨 High Priority:

1. File permissions documentation: Add a note in documentation about securing the lastfm.conf file with appropriate permissions (600/700)

🔍 Medium Priority:

2. Memory security: Consider using secure memory allocation for password strings
3. Config validation: Add validation for session key format to detect corruption

📝 Low Priority:

4. Logging consistency: Consider adding more structured logging with log levels instead of channel-based filtering

Final Assessment

✅ APPROVED - This is a well-executed security fix that:

• Addresses the stated vulnerability completely
• Maintains code functionality and style
• Includes appropriate CI/CD updates
• Has good existing test coverage
• Makes minimal, targeted changes

The information leak vulnerability has been properly addressed without introducing new issues.

--- | Branch: fix-lastfm-info-leak-17259440130858798210

[github-actions]

Claude encountered an error —— View job

---

I'll analyze this and get back to you.

[github-actions]

Claude encountered an error —— View job

---

I'll analyze this and get back to you.