Merged
Conversation
Pull Assistants update
Update docker hub branch to nightly, custom branches will be needed to archive releases
CRITICAL FIXES: - Fix database credentials bug in settings.py (lines 65-78) - variables were assigned args.db_host instead of correct arguments - Fix path traversal vulnerability in audioCog.py - added validation to prevent directory traversal attacks - Replace deprecated .logout() with .close() in adminCog.py for discord.py 2.0+ compatibility - Fix unsafe admin restart logic - added note about process manager requirement - Improve exception handling in database operations - use specific exceptions instead of bare except HIGH PRIORITY FIXES: - Implement blacklist persistence in openAiCog.py - blacklist now saves to/loads from JSON file - Fix file handle leak in openAiCog.py - use context manager for file operations - Add IndexError protection in openAiCog.py - check attachments list before accessing - Fix ValueError in blacklist removal - check if user exists before removing - Fix fragile string comparison in run status check - use exact equality - Fix incorrect task loop parameters in generalCog.py - use @tasks.loop(hours=1) - Fix text channel attribute access in audioCog.py - handle both Context and Channel objects - Fix blocking ffmpeg.probe() operation - run in executor to avoid blocking event loop MEDIUM PRIORITY FIXES: - Remove debug print statement in twitterCog.py - replace with logger.debug() - Add error handling for Twitter API and wget calls - Fix improper string formatting in gameCog.py logging - Add input validation for gameCog commands - validate int conversions - Remove admin list leak from unauthorized access messages - Add rate limiting for OpenAI API calls - 1 request per 60 seconds per user - Add TTS text length validation - max 500 characters - Add repeat command limit - max 10 repeats All fixes follow security best practices and improve code reliability.
- Replace hardcoded 'nightly' tag with docker/metadata-action - Automatically tags Docker images based on branch name - Supports branch pushes, PRs, and SHA-based tags - Adds required checkout step for metadata action
…zDQSmjB2LFn29M Pull Claude changes
Ian nightly
This commit resolves 8 critical issues identified in the comprehensive code review: 1. **Resolve merge conflict in openAiCog.py** - Fixed unresolved Git merge conflict markers in edit_img() function - Properly merged cooldown decorator and context manager usage - Fixed typo: blackisted → blacklisted 2. **Fix blocking I/O in async context** - Changed time.sleep(5) to await asyncio.sleep(5) in voiceCog.py - Prevents event loop blocking during voice clip generation 3. **Move hardcoded IP to configuration** - Replaced hardcoded 192.168.1.230:8000 with configurable voice_api_url - Added fallback to localhost for development environments 4. **Move secrets to environment variables** - Added support for DISCORD_BOT_TOKEN, DB_PASSWORD, and OPENAI_API_KEY - Maintains backward compatibility with JSON config - Improves security by supporting environment-based secrets 5. **Fix resource leak - unclosed file handles** - Changed file opening in voiceCog.py to use context manager - Prevents file descriptor leaks over time 6. **Fix privacy issue - reduce message logging** - Modified generalCog.py to only log command messages - Reduces GDPR/privacy concerns and excessive log file growth - Prevents logging of all user messages 7. **Implement empty admin commands** - Added functionality to hash() and version() commands - Both commands now use git to retrieve commit hash and version tag - Added proper admin permission checks and error handling 8. **Add input validation for URLs and filenames** - Added URL validation for youtube/stream commands (whitelist approach) - Added filename sanitization to prevent path traversal attacks - Prevents malicious URLs and filenames from being processed Files changed: - cogs/openAiCog.py: Merge conflict resolved, env var support - cogs/voiceCog.py: Async sleep, configurable API URL, file handle fix - cogs/adminCog.py: Implemented hash() and version() commands - cogs/audioCog.py: URL and filename validation - cogs/generalCog.py: Privacy-conscious logging - settings.py: Environment variable support for secrets
This commit addresses critical issues identified in the second code review: 1. **CRITICAL: Fix admin authentication vulnerability** (adminCog.py) - Changed from string comparison to Discord user ID checks - Prevents username spoofing attacks - Added backward compatibility with existing "admins" config - New config option "admin_ids" for secure ID-based auth - Applied to: hash(), version(), kill(), restart() commands 2. **CRITICAL: Add request timeouts to prevent DoS** (voiceCog.py) - Added 30-second timeout to all HTTP requests - Prevents bot from hanging indefinitely on network issues - Applied to: add_voice, add_clip, make_clip, list_voices commands - Protects against DoS attacks via slow HTTP responses 3. **CRITICAL: Move Twitter credentials to env variables** (twitterCog.py) - Added support for TWITTER_API_KEY, TWITTER_API_SECRET - Added support for TWITTER_ACCESS_TOKEN, TWITTER_ACCESS_SECRET - Maintains backward compatibility with JSON config - Added null-check before using Twitter API 4. **CRITICAL BUG: Fix division by zero in teams command** (gameCog.py) - Fixed crash when more teams requested than players available - Properly distributes players across teams with integer division - Handles extra players by distributing to first teams - Added edge case validation and clear error messages 5. **MAJOR: Fix deleted message privacy issue** (generalCog.py) - Changed from logging full message content to metadata only - Logs: author, channel, timestamp, message length - Prevents GDPR/privacy violations - Reduces log file size and privacy concerns 6. **MAJOR BUG: Fix empty status array crash** (generalCog.py) - Added check for empty status array before random selection - Uses random.choice() instead of randint() for cleaner code - Prevents IndexError when status list is empty - Logs warning if no statuses configured Security improvements: - Admin commands now use Discord IDs (cannot be spoofed) - All HTTP requests have timeouts (prevents DoS) - Twitter API credentials support environment variables - Privacy-conscious logging (no deleted message content) Stability improvements: - Fixed division by zero crash in teams command - Fixed empty array crash in status rotation - Better error handling and edge case validation - Improved team distribution algorithm Files changed: - cogs/adminCog.py: Secure admin authentication - cogs/voiceCog.py: Request timeouts for all HTTP calls - cogs/twitterCog.py: Environment variable support - cogs/gameCog.py: Fixed team division algorithm - cogs/generalCog.py: Privacy-conscious logging, empty array fix
This commit addresses the final 4 critical issues identified in the security audit:
1. **CRITICAL-SECURITY: Path Traversal in MP3 Upload Handler** (audioCog.py:195-240)
- VULNERABILITY: Attackers could upload files with names like "../../etc/cron.d/malicious.mp3"
- IMPACT: Arbitrary file write, potential remote code execution, system compromise
- FIX:
* Use existing sanitize_filename() method to remove path components
* Validate file extension (.mp3 only)
* Validate absolute paths don't escape soundboard directories
* Added logging for path traversal attempts
* Apply validation to both raw/ and final soundboard/ directories
2. **CRITICAL-SECURITY: Path Traversal in TTS Command** (audioCog.py:585-621)
- VULNERABILITY: .say command didn't validate tts_file parameter
- IMPACT: Arbitrary file write via TTS audio generation
- EXPLOIT: ".say hello ../../tmp/pwned" writes to ./tmp/pwned.mp3
- FIX:
* Sanitize tts_file parameter using sanitize_filename()
* Validate absolute path stays within soundboard directory
* Handle .mp3 extension properly (remove if provided by user)
* Added logging for path traversal attempts
3. **CRITICAL-SECURITY: Authentication Bypass in OpenAI Admin Commands** (openAiCog.py:508-549)
- VULNERABILITY: openai_ban and openai_unban used insecure string-based auth
- IMPACT: Username spoofing, unauthorized ban/unban access
- FIX:
* Changed to Discord user ID-based authentication (same as adminCog.py)
* Check admin_ids first (secure), fallback to admins (backward compat)
* Added logging for unauthorized attempts
* Consistent error messages with other admin commands
4. **CRITICAL-BUG: Webhook Handler IndexError Crash** (audioCog.py:243-262)
- BUG: Webhook parser accessed array elements without length check
- IMPACT: Bot crashes on malformed webhook messages, DoS vulnerability
- EXPLOIT: Send "www.sodersjerna.com" without colons → IndexError
- FIX:
* Added length check: len(data) >= 4 before accessing elements
* Prevents IndexError on malformed webhook messages
* Maintains backward compatibility with valid webhooks
Security improvements:
- Path traversal attacks blocked on file uploads and TTS generation
- Admin authentication now uses Discord IDs (cannot be spoofed)
- Webhook handler is crash-proof against malformed input
- All attacks logged with attacker identification
Stability improvements:
- No more crashes from malformed webhook messages
- Proper validation at all file operation boundaries
- Consistent error handling and user feedback
Files changed:
- cogs/audioCog.py: Path traversal fixes, webhook crash fix
- cogs/openAiCog.py: Secure admin authentication
All critical security vulnerabilities are now resolved.
This commit addresses multiple critical and major security/stability issues identified in the code review: ## Critical Security Fixes (CS): - **CS-1**: Implement missing get_blacklists() database function that was returning None, causing potential authentication bypass - **CS-2**: Fix SSRF vulnerability in voice API by adding URL validation, parameter sanitization for voice_name and UUID inputs - **CS-3**: Add comprehensive error handling to all OpenAI database operations with connection validation and retry logic ## Critical Crash Bugs (CB): - **CB-1**: Add proper error handling for JSON file loading (FileNotFoundError, JSONDecodeError, PermissionError) - **CB-2**: Add error handling for cogs directory operations with proper validation and individual cog load error handling - **CB-3**: Add comprehensive error handling for YouTube directory cleanup operations ## Critical Stability (CST): - **CST-1**: Refactor event loop blocking in OpenAI assistant by wrapping all synchronous OpenAI API calls in asyncio.to_thread() ## Major Bugs (MB): - **MB-1**: Fix list modification during iteration bug in verify_db() by using list comprehensions and set operations ## Technical Details: - voiceCog.py: Added URL validation, regex-based input sanitization, URL encoding for all API parameters - openAiCog.py: Wrapped all blocking OpenAI client calls (threads.messages, threads.runs, files.content) in asyncio.to_thread - openai_database_manager.py: Implemented get_blacklists(), added connection validation, IntegrityError handling, and retry logic - settings.py: Added try/except for JSON loading with helpful error messages, fixed list iteration bug in verify_db() - BotHead.py: Added comprehensive error handling for cogs directory with per-cog load error handling - audioCog.py: Added error handling for YouTube cleanup with directory validation and file operation error handling All changes maintain backward compatibility while significantly improving security and stability.
plop91
changed the title
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
2 participants
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.
Pull devel up