From be34ea1344d3e03cf3a6bb0b844f5d5ebd514309 Mon Sep 17 00:00:00 2001 From: Rob Jarawan <32302742+robjarawan@users.noreply.github.com> Date: Wed, 22 Apr 2026 23:01:48 -0400 Subject: [PATCH] Fix rabbitmq_admin __main__ using stdlib urlparse for percent-encoded passwords After commit 72a8d193 removed unquote() calls from exec_rabbitmqadmin, the function relies on UrlParseResult.password to return the decoded value. The __main__ block used urllib.parse.urlparse() (plain ParseResult) which does not decode percent-encoding, so passwords like 'pass%23word' were passed verbatim to rabbitmqadmin instead of 'pass#word', breaking auth for anyone invoking the script directly with an encoded password. Switch __main__ to _urlparse() so url.password returns the decoded value consistently with the non-main code paths. Add three tests: two confirming decoded passwords reach the subprocess, one pinning the stdlib urlparse behaviour to document why _urlparse is needed. --- sarracenia/rabbitmq_admin.py | 3 +- tests/sarracenia/rabbitmq_admin_test.py | 48 +++++++++++++++++++++++-- 2 files changed, 48 insertions(+), 3 deletions(-) diff --git a/sarracenia/rabbitmq_admin.py b/sarracenia/rabbitmq_admin.py index ed25201d1..997742932 100755 --- a/sarracenia/rabbitmq_admin.py +++ b/sarracenia/rabbitmq_admin.py @@ -291,7 +291,8 @@ def user_access(url, user): if __name__ == "__main__": - url = urllib.parse.urlparse(sys.argv[1]) + from sarracenia.config.credentials import _urlparse + url = _urlparse(sys.argv[1]) print(exec_rabbitmqadmin(url, "list queue names")[1]) import json diff --git a/tests/sarracenia/rabbitmq_admin_test.py b/tests/sarracenia/rabbitmq_admin_test.py index 0de998cd0..dcf62c149 100644 --- a/tests/sarracenia/rabbitmq_admin_test.py +++ b/tests/sarracenia/rabbitmq_admin_test.py @@ -1,6 +1,50 @@ import pytest from tests.conftest import * -#from unittest.mock import Mock +from unittest.mock import patch, MagicMock import sarracenia.config -import sarracenia.rabbitmq_admin \ No newline at end of file +import sarracenia.rabbitmq_admin +from sarracenia.rabbitmq_admin import exec_rabbitmqadmin +from sarracenia.config.credentials import _urlparse + + +class Test_ExecRabbitmqadmin: + """Regression tests for PR #989 / #1677 credential handling in rabbitmq_admin.""" + + def test_plain_password_reaches_command(self): + """Plain password is passed to exec_rabbitmqadmin without modification.""" + url = _urlparse('amqp://admin:secret@localhost/') + with patch('sarracenia.rabbitmq_admin.subprocess.run') as mock_run: + mock_run.return_value = MagicMock(returncode=0, stdout=b'[]') + exec_rabbitmqadmin(url, 'list exchanges name') + call_args = mock_run.call_args[0][0] + assert 'secret' in call_args + + def test_encoded_hash_password_decoded_for_command(self): + """Password with '%23' must be decoded to '#' before being passed to rabbitmqadmin. + + Regression: exec_rabbitmqadmin used urllib.parse.urlparse() in __main__ + (plain ParseResult) and then dropped the unquote() call, so percent-encoded + passwords were passed verbatim ('pass%23word') instead of decoded ('pass#word'). + """ + url = _urlparse('amqp://admin:pass%23word@localhost/') + assert url.password == 'pass#word', "UrlParseResult must decode %23 to #" + + with patch('sarracenia.rabbitmq_admin.subprocess.run') as mock_run: + mock_run.return_value = MagicMock(returncode=0, stdout=b'[]') + exec_rabbitmqadmin(url, 'list exchanges name') + call_args = mock_run.call_args[0][0] + # Decoded password must appear; encoded form must not + assert 'pass#word' in ' '.join(call_args) or 'pass#word' in call_args + assert 'pass%23word' not in ' '.join(call_args) + + def test_plain_urlparse_would_pass_encoded_password(self): + """Demonstrates the bug: stdlib urlparse returns encoded password from url.password. + + This test documents why the __main__ block must use _urlparse, not urlparse. + Plain ParseResult.password does NOT decode percent-encoding. + """ + import urllib.parse + url_broken = urllib.parse.urlparse('amqp://admin:pass%23word@localhost/') + assert url_broken.password == 'pass%23word', \ + "stdlib urlparse must NOT decode — confirms why _urlparse is needed in __main__"