chore: set password on embedded Redis instance#41634
Open
wyattwalter wants to merge 1 commit intoreleasefrom
Open
chore: set password on embedded Redis instance#41634wyattwalter wants to merge 1 commit intoreleasefrom
wyattwalter wants to merge 1 commit intoreleasefrom
Conversation
Generate a random password for the embedded Redis when running locally, following the same pattern used for MongoDB, Postgres, and Supervisor credentials. Existing installs are backfilled on next startup. The Redis server config is written to a file rather than passed via CLI args to keep the password out of the process list. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
github-actions
bot
added
the
skip-changelog
Contributor
WalkthroughIntroduces Redis password generation and management for Docker deployments. Generates a password during initial configuration, stores it in environment files, embeds it in Redis connection strings for localhost setups, and creates temporary Redis configuration files with restrictive permissions for local Redis instances. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@deploy/docker/fs/opt/appsmith/entrypoint.sh`:
- Around line 115-126: Treat empty APPSMITH_REDIS_PASSWORD as missing and when
backfilling a generated_appsmith_redis_password, preserve the existing
APPSMITH_REDIS_URL host/port/path/query instead of hardcoding 127.0.0.1:6379:
check for APPSMITH_REDIS_PASSWORD being unset or empty, generate
generated_appsmith_redis_password as you already do, append it to the env, then
parse current_redis_url (from the APPSMITH_REDIS_URL line) and inject the
password into that URL by inserting :<generated_appsmith_redis_password>@ after
the scheme (e.g. redis://) while keeping the original host/port/path/query
intact; update the APPSMITH_REDIS_URL line with the modified URL rather than
replacing it with a fixed host.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 11ec7dbe-8668-44d9-b4ec-76a5916bf71e
📒 Files selected for processing (3)
deploy/docker/fs/opt/appsmith/entrypoint.shdeploy/docker/fs/opt/appsmith/templates/docker.env.shdeploy/docker/fs/opt/appsmith/templates/supervisord/redis.conf
Comment on lines
+115
to
+126
| if ! grep -q "APPSMITH_REDIS_PASSWORD" "$ENV_PATH"; then | ||
| local generated_appsmith_redis_password=$( | ||
| tr -dc A-Za-z0-9 </dev/urandom | head -c 13 | ||
| echo '' | ||
| ) | ||
| echo $'\nAPPSMITH_REDIS_PASSWORD='"$generated_appsmith_redis_password" >> "$ENV_PATH" | ||
| # Update the Redis URL to include the password, but only for the embedded Redis. | ||
| local current_redis_url | ||
| current_redis_url=$(grep "^APPSMITH_REDIS_URL=" "$ENV_PATH" | tail -1 | cut -d= -f2-) | ||
| if [[ "$current_redis_url" == *"localhost"* || "$current_redis_url" == *"127.0.0.1"* ]]; then | ||
| sed -i "s|^APPSMITH_REDIS_URL=.*|APPSMITH_REDIS_URL=redis://:${generated_appsmith_redis_password}@127.0.0.1:6379|" "$ENV_PATH" | ||
| fi |
Contributor
There was a problem hiding this comment.
Preserve existing localhost Redis URL details during backfill.
Line 125 rewrites to a fixed 127.0.0.1:6379 URL, which can break upgrades where local Redis used a custom port/path/query. Also, Line 115 should treat empty APPSMITH_REDIS_PASSWORD as missing.
Proposed fix
- if ! grep -q "APPSMITH_REDIS_PASSWORD" "$ENV_PATH"; then
+ if ! grep -qE '^APPSMITH_REDIS_PASSWORD=.+$' "$ENV_PATH"; then
local generated_appsmith_redis_password=$(
tr -dc A-Za-z0-9 </dev/urandom | head -c 13
echo ''
)
echo $'\nAPPSMITH_REDIS_PASSWORD='"$generated_appsmith_redis_password" >> "$ENV_PATH"
# Update the Redis URL to include the password, but only for the embedded Redis.
local current_redis_url
+ local redis_target
current_redis_url=$(grep "^APPSMITH_REDIS_URL=" "$ENV_PATH" | tail -1 | cut -d= -f2-)
if [[ "$current_redis_url" == *"localhost"* || "$current_redis_url" == *"127.0.0.1"* ]]; then
- sed -i "s|^APPSMITH_REDIS_URL=.*|APPSMITH_REDIS_URL=redis://:${generated_appsmith_redis_password}@127.0.0.1:6379|" "$ENV_PATH"
+ redis_target="${current_redis_url#redis://}" # strip scheme
+ redis_target="${redis_target#*@}" # strip existing credentials if present
+ sed -i "s|^APPSMITH_REDIS_URL=.*|APPSMITH_REDIS_URL=redis://:${generated_appsmith_redis_password}@${redis_target}|" "$ENV_PATH"
fi
fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if ! grep -q "APPSMITH_REDIS_PASSWORD" "$ENV_PATH"; then | |
| local generated_appsmith_redis_password=$( | |
| tr -dc A-Za-z0-9 </dev/urandom | head -c 13 | |
| echo '' | |
| ) | |
| echo $'\nAPPSMITH_REDIS_PASSWORD='"$generated_appsmith_redis_password" >> "$ENV_PATH" | |
| # Update the Redis URL to include the password, but only for the embedded Redis. | |
| local current_redis_url | |
| current_redis_url=$(grep "^APPSMITH_REDIS_URL=" "$ENV_PATH" | tail -1 | cut -d= -f2-) | |
| if [[ "$current_redis_url" == *"localhost"* || "$current_redis_url" == *"127.0.0.1"* ]]; then | |
| sed -i "s|^APPSMITH_REDIS_URL=.*|APPSMITH_REDIS_URL=redis://:${generated_appsmith_redis_password}@127.0.0.1:6379|" "$ENV_PATH" | |
| fi | |
| if ! grep -qE '^APPSMITH_REDIS_PASSWORD=.+$' "$ENV_PATH"; then | |
| local generated_appsmith_redis_password=$( | |
| tr -dc A-Za-z0-9 </dev/urandom | head -c 13 | |
| echo '' | |
| ) | |
| echo $'\nAPPSMITH_REDIS_PASSWORD='"$generated_appsmith_redis_password" >> "$ENV_PATH" | |
| # Update the Redis URL to include the password, but only for the embedded Redis. | |
| local current_redis_url | |
| local redis_target | |
| current_redis_url=$(grep "^APPSMITH_REDIS_URL=" "$ENV_PATH" | tail -1 | cut -d= -f2-) | |
| if [[ "$current_redis_url" == *"localhost"* || "$current_redis_url" == *"127.0.0.1"* ]]; then | |
| redis_target="${current_redis_url#redis://}" # strip scheme | |
| redis_target="${redis_target#*@}" # strip existing credentials if present | |
| sed -i "s|^APPSMITH_REDIS_URL=.*|APPSMITH_REDIS_URL=redis://:${generated_appsmith_redis_password}@${redis_target}|" "$ENV_PATH" | |
| fi |
🧰 Tools
🪛 Shellcheck (0.11.0)
[warning] 116-116: Declare and assign separately to avoid masking return values.
(SC2155)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@deploy/docker/fs/opt/appsmith/entrypoint.sh` around lines 115 - 126, Treat
empty APPSMITH_REDIS_PASSWORD as missing and when backfilling a
generated_appsmith_redis_password, preserve the existing APPSMITH_REDIS_URL
host/port/path/query instead of hardcoding 127.0.0.1:6379: check for
APPSMITH_REDIS_PASSWORD being unset or empty, generate
generated_appsmith_redis_password as you already do, append it to the env, then
parse current_redis_url (from the APPSMITH_REDIS_URL line) and inject the
password into that URL by inserting :<generated_appsmith_redis_password>@ after
the scheme (e.g. redis://) while keeping the original host/port/path/query
intact; update the APPSMITH_REDIS_URL line with the modified URL rather than
replacing it with a fixed host.
Contributor
Author
|
/build-deploy-preview |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/23296377431. |
|
Deploy-Preview-URL: https://ce-41634.dp.appsmith.com |
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
1 participant
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
Test plan
APPSMITH_REDIS_PASSWORDand updatedAPPSMITH_REDIS_URLappear indocker.envredis-cliwithout-a <password>is rejected, with password works🤖 Generated with Claude Code
Summary by CodeRabbit