fix(celery): percent-encode the broker credentials - #60
Conversation
Interpolating the RabbitMQ password straight into an amqp URL breaks when it contains a '#', '?' or '/': those terminate the netloc, so the port parses as whatever trailed the last colon and the connection dies with "Port could not be cast to integer value". '@' and ':' survive naive interpolation, which is why this held up until a password happened to contain one of the other three. Encode user, password, and vhost instead. The event_bus consumers were never exposed to this because pika takes credentials as separate parameters. Celery is the first thing here to parse them as a URL, so the fault only surfaced once a worker ran. Also stop an unreachable broker from failing the write. Outside an atomic block Django runs an on_commit callback inline, so the enqueue happens inside the request and any broker fault reaches the client as a 500 on task creation. The record is already committed as pending and the periodic sweep re-queues it, so log the failure and carry on: syncing late is better than refusing to save.
|
| GitGuardian id | GitGuardian status | Secret | Commit | Filename | |
|---|---|---|---|---|---|
| 35216851 | Triggered | Generic Password | bba1c4b | keep_up/test_broker_url.py | View secret |
| 35216848 | Triggered | Generic Password | bba1c4b | keep_up/test_broker_url.py | View secret |
| 35216850 | Triggered | Generic Password | bba1c4b | keep_up/test_broker_url.py | View secret |
| 35216848 | Triggered | Generic Password | bba1c4b | keep_up/test_broker_url.py | View secret |
| 35216851 | Triggered | Generic Password | bba1c4b | keep_up/test_broker_url.py | View secret |
| 35216850 | Triggered | Generic Password | bba1c4b | keep_up/test_broker_url.py | View secret |
| 35216849 | Triggered | Generic Password | bba1c4b | keep_up/test_broker_url.py | View secret |
| 35216849 | Triggered | Generic Password | bba1c4b | keep_up/test_broker_url.py | View secret |
🛠 Guidelines to remediate hardcoded secrets
- Understand the implications of revoking this secret by investigating where it is used in your code.
- Replace and store your secrets safely. Learn here the best practices.
- Revoke and rotate these secrets.
- If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.
To avoid such incidents in the future consider
- following these best practices for managing and storing secrets including API keys and other credentials
- install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.
🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 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 |
1 participant
Interpolating the RabbitMQ password straight into an amqp URL breaks when it contains a '#', '?' or '/': those terminate the netloc, so the port parses as whatever trailed the last colon and the connection dies with "Port could not be cast to integer value". '@' and ':' survive naive interpolation, which is why this held up until a password happened to contain one of the other three. Encode user, password, and vhost instead.
The event_bus consumers were never exposed to this because pika takes credentials as separate parameters. Celery is the first thing here to parse them as a URL, so the fault only surfaced once a worker ran.
Also stop an unreachable broker from failing the write. Outside an atomic block Django runs an on_commit callback inline, so the enqueue happens inside the request and any broker fault reaches the client as a 500 on task creation. The record is already committed as pending and the periodic sweep re-queues it, so log the failure and carry on: syncing late is better than refusing to save.