-
-
Notifications
You must be signed in to change notification settings - Fork 13
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix MySQL commands #354
Fix MySQL commands #354
Conversation
WalkthroughThe changes in the Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
bookstack/rootfs/etc/cont-init.d/bookstack.sh (2)
50-54
: Consider security implications of disabling SSLWhile the change from
mysql
tomariadb
command is appropriate, disabling SSL (--skip-ssl
) for local connections might need reconsideration:
- Even for local connections, SSL provides an additional security layer.
- Instead of disabling SSL completely, consider using proper SSL certificates.
Consider these alternatives:
- Use SSL certificates provided by the MariaDB addon
- If SSL must be disabled, add a comment explaining why it's safe in this context (e.g., if the connection is guaranteed to be local only)
50-54
: Enhance database creation commandThe database creation command could be improved for better security and clarity.
Consider this enhanced version:
mariadb \ -u "${username}" -p"${password}" \ -h "${host}" -P "${port}" \ --skip-ssl \ - -e "CREATE DATABASE IF NOT EXISTS \`bookstack\` ;" + -e "CREATE DATABASE IF NOT EXISTS \`bookstack\` CHARACTER SET utf8mb4 COLLATE utf8mb4_unicode_ci;"This change:
- Explicitly sets the character set to utf8mb4 for full Unicode support
- Sets a proper collation for consistent string comparisons
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
bookstack/rootfs/etc/cont-init.d/bookstack.sh
(1 hunks)
🔇 Additional comments (1)
bookstack/rootfs/etc/cont-init.d/bookstack.sh (1)
50-54
: Verify other MySQL command usage
Let's ensure all MySQL commands in the codebase are updated consistently.
✅ Verification successful
MySQL command usage is consistent
Based on the search results, there is only one MySQL/MariaDB command execution in the codebase, which is the one being reviewed. The --skip-ssl
flag addition is isolated to this single database creation command. The rest of the database connectivity is handled through environment variables and PHP database drivers, which manage their own SSL settings separately.
- The only direct database command is in
bookstack/rootfs/etc/cont-init.d/bookstack.sh
for database creation - Database connection settings for the application are managed via environment variables in
bookstack/rootfs/etc/services.d/php-fpm/run
- No other MySQL/MariaDB command executions were found that would need similar updates
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining mysql commands that might need updates
rg -i "mysql\s+[^-]|mysql$" --type sh
# Search for database connection configurations that might need SSL settings
rg -i "DB_|database|mysql|mariadb" --type php --type env
Length of output: 126
Script:
#!/bin/bash
# Search for any remaining mysql/mariadb commands in shell scripts
rg -i "mysql|mariadb" --type sh -A 3
# Search for database configuration files and environment templates
fd -e php -e env.example -e env.sample -e env.template | xargs rg -i "DB_|database|mysql|mariadb"
# Search specifically in the bookstack directory for any database related files
fd . bookstack/ -t f | xargs rg -i "mysql|mariadb|database"
Length of output: 13133
Proposed Changes
Fixes the MySQL/MariaDB commands.
With the latest MariaDB CLI, SSL is enabled by default. Additionally, calling the CLI using
mysql
is now deprecated.Summary by CodeRabbit
New Features
Bug Fixes
Chores
mysql
tomariadb
with new options.