Skip to content

fix: preserve # in pg_service.conf passwords#1575

Open
lawrence3699 wants to merge 1 commit intodbcli:mainfrom
lawrence3699:fix/pg-service-password-hash
Open

fix: preserve # in pg_service.conf passwords#1575
lawrence3699 wants to merge 1 commit intodbcli:mainfrom
lawrence3699:fix/pg-service-password-hash

Conversation

@lawrence3699
Copy link
Copy Markdown

Fixes #1512

The service-file parser currently uses ConfigObj, which treats # as an inline comment in .pg_service.conf. That truncates entries like password=abc#123 to abc, so connect_service() passes the wrong password down to the connection layer.

This switches only the .pg_service.conf parser to ConfigParser(interpolation=None). It keeps the existing initial-comment skipping behavior and preserves parse-error line numbers by padding the skipped lines back before parsing.

Validation:

  • pytest tests/test_main.py -k 'pg_service_file' -q
  • pytest tests/test_config.py -q
  • ruff check pgcli/main.py tests/test_main.py

Copilot AI review requested due to automatic review settings April 18, 2026 03:11
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Updates pgcli’s .pg_service.conf parsing so passwords containing # are preserved correctly (fixing #1512), aligning connect_service() behavior with psql for these values.

Changes:

  • Switch .pg_service.conf parsing from ConfigObj to configparser.ConfigParser(interpolation=None) and preserve error line numbers by padding skipped lines.
  • Add a regression test ensuring password=abc#123 is passed through unchanged.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
pgcli/main.py Replaces the service-file parser implementation to avoid treating # as an inline comment.
tests/test_main.py Adds a regression test for service passwords containing #.

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

Comment thread tests/test_main.py
Comment on lines +523 to +533
os.environ["PGSERVICEFILE"] = tmpdir.join(".pg_service.conf").strpath
cli.connect_service("myservice", None)
mock_connect.assert_called_with(
database="a_dbname",
host="a_host",
user="a_user",
port="5433",
passwd="abc#123",
)

del os.environ["PGSERVICEFILE"]
Copy link

Copilot AI Apr 18, 2026

Choose a reason for hiding this comment

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

The test manually sets/unsets os.environ["PGSERVICEFILE"] and deletes it at the end, but if an assertion fails the cleanup won’t run and can leak state into later tests. Prefer using the monkeypatch fixture (or a try/finally) to set the env var so it’s always restored.

Copilot uses AI. Check for mistakes.
bobo-xxx added a commit to bobo-xxx/pgcli that referenced this pull request Apr 18, 2026
The # character was being treated as a comment delimiter by ConfigObj,
truncating passwords like abc#123 to abc. Added comment_tokens=[] to
disable this behavior.

Fixes dbcli#1575
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.

pgcli couldn't parse password that contained character '#'

2 participants