Skip to content

fix(docker): keep the Redis password out of the RQ worker command line - #4093

Merged
gantoine merged 1 commit into
masterfrom
fix/rq-worker-redis-url-env
Aug 4, 2026
Merged

fix(docker): keep the Redis password out of the RQ worker command line#4093
gantoine merged 1 commit into
masterfrom
fix/rq-worker-redis-url-env

Conversation

@gantoine

@gantoine gantoine commented Aug 4, 2026

Copy link
Copy Markdown
Member

Description

The RQ worker received its Redis connection string through rq worker --url, which placed the fully-authenticated URL (including REDIS_PASSWORD) into the process's argv for the life of the worker. Command-line arguments are readable via world-readable /proc/<pid>/cmdline, ps, container diagnostics, and any monitoring agent or support bundle that collects process listings (CWE-214). Operators pointing RomM at an external password-authenticated Redis/Valkey/Dragonfly were exposing that credential to anyone with process-inspection access in the container.

RQ's --url option already declares envvar='RQ_REDIS_URL', so passing the URL through the environment instead is behavior-preserving and needs no version bump. This also brings the worker in line with the rqscheduler started next to it, which was already taking its credentials as RQ_REDIS_* environment variables.

Fixed in both places the worker is launched:

  • docker/init_scripts/init (production container)
  • entrypoint.sh (dev-mode compose entrypoint, which had the same pattern)

The credential still lives in the process environment, visible in /proc/<pid>/environ. That is a meaningful narrowing rather than a full elimination: environ is restricted to the same UID or root, unlike world-readable cmdline, and it adds no new exposure because REDIS_PASSWORD is already in the container environment and inherited by every child process. Removing environment-based exposure entirely would require a credentials file or an --config module, which is a larger change than this fix warrants.

Verification

  • Confirmed RQ honors the env var: ran RQ_REDIS_URL="redis://:PW@127.0.0.1:6399/0" rq worker with no --url flag and it attempted 127.0.0.1:6399, a port that could only have come from the environment (the default is 6379).
  • bash -n on both scripts, plus trunk fmt and trunk check clean.

I did not rebuild the container to re-run the original reproduction against a patched image; the CLI-level check above establishes that the flag is read from the environment, and the password is absent from argv by construction.

AI assistance disclosure

This change was written with AI assistance (Claude Code). The report was reviewed and confirmed against the source, the fix and its verification were AI-authored, and I reviewed the result before submitting.

Checklist

  • I've tested the changes locally
  • I've updated relevant comments
  • I've assigned reviewers for this PR
  • I've added unit tests that cover the changes

No unit tests added: these are container startup shell scripts with no existing test harness in the repo.

The worker received its connection string through `rq worker --url`, which
put the password in argv for the life of the process, readable via
/proc/<pid>/cmdline, ps, and any monitoring or diagnostic tooling that
collects command lines (CWE-214).

RQ's `--url` option already reads from RQ_REDIS_URL, so passing it through
the environment instead is behavior-preserving. This matches how the RQ
scheduler alongside it already takes its credentials.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings August 4, 2026 01:24
@greptile-apps

greptile-apps Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR removes authenticated Redis URLs from RQ worker command-line arguments while preserving worker connection behavior through RQ’s supported environment variable.

  • Passes the production worker URL through RQ_REDIS_URL in docker/init_scripts/init.
  • Applies the same credential-exposure reduction to the development worker in entrypoint.sh.
  • Leaves scheduler configuration and Redis URL construction unchanged.

Confidence Score: 5/5

The PR appears safe to merge, with both worker launch paths preserving Redis connection behavior while removing credentials from process arguments.

The command-scoped environment assignments reach the RQ worker directly, and the supported RQ_REDIS_URL input receives the same constructed URL previously passed through --url; no actionable regression remains.

Reviews (1): Last reviewed commit: "fix(docker): keep the Redis password out..." | Re-trigger Greptile

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates RomM’s container startup scripts to avoid passing the authenticated Redis URL (including REDIS_PASSWORD) via rq worker --url, which exposed credentials in the worker process argv. Instead, it relies on RQ’s supported environment variable (RQ_REDIS_URL) to keep credentials out of the world-readable command line while preserving existing connection behavior.

Changes:

  • Switch rq worker startup from --url "${...}" to RQ_REDIS_URL="${...}" (env var) in both prod and dev entrypoints.
  • Add inline documentation explaining the security rationale (CWE-214 style argv exposure).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
entrypoint.sh Starts the dev-mode RQ worker using RQ_REDIS_URL instead of --url to keep the password out of argv.
docker/init_scripts/init Starts the production RQ worker using RQ_REDIS_URL instead of --url to keep the password out of argv.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@gantoine
gantoine merged commit 985fefb into master Aug 4, 2026
10 checks passed
@gantoine
gantoine deleted the fix/rq-worker-redis-url-env branch August 4, 2026 01:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants