Reject empty or null Username on non-shared server import - #10320
Conversation
…in-org#10309) validate_json_data() only checked that the Username key was present on a non-shared server, not that it held anything useful, so an empty or null value imported cleanly and left behind a server that libpq would silently authenticate as the OS account running pgAdmin rather than reject outright. Check the value, matching the truthiness check already used for shared servers and the "Username must be specified" rule enforced by the server dialog.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 8 included reviews per hour; 0 remain after this review. WalkthroughNon-shared server validation now rejects empty or null ChangesServer import username validation
Estimated code review effort: 1 (Trivial) | ~5 minutes Merge Risk: ⚪ Minimal · up to This localized validation change rejects unusable empty or null usernames during non-shared server import, preventing misleading connection failures; no actionable merge-blocking risk remains after normal checks and review. Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
Summary
validate_json_data()only checked that theUsernamekey was presenton a non-shared server entry in
servers.json, not that it held anythinguseful.
{"Username": ""}and{"Username": null}both importedcleanly and left behind a server that couldn't sensibly connect: libpq
treats an empty
useras "unset" and substitutes the OS account runningpgAdmin, so the resulting authentication failure has no obvious link
back to the servers.json that caused it. The server dialog already
rejects an empty username ("Username must be specified."); import was
more permissive than the UI it's meant to mirror.
This checks the value rather than just the key, matching the truthiness
check already used for the shared-server
Username/SharedUsernamepair a few lines above.
Fixes #10309.
Test plan
test_validate_json_data.pycovering an emptyand a null
Usernameon a non-shared server (both now rejected).python regression/runtests.py --pkg utils.tests.test_validate_json_datapasses (14/14).
Summary by CodeRabbit
Bug Fixes
Tests