From eff66cc6c614bc302f5edb168ab0e602bbffe0dd Mon Sep 17 00:00:00 2001
From: Kundan Sable <kundan.sable@enterprisedb.com>
Date: Thu, 23 Jul 2026 12:37:39 +0530
Subject: [PATCH 1/4] fix: persist rotated saved password and correct manager
 session state

For a server configured to save its password, entering a rotated
password at the reconnect/password-prompt dialog appeared to work
for the current connection but never actually replaced the stale
saved ciphertext, causing an infinite re-prompt loop on every
subsequent connection (e.g. opening the Query Tool).

Two gaps caused this:

1. Query Tool's "already connected" reconnect path
   (sqleditor.connect_server) cached the freshly entered password on
   the in-memory server manager only, never writing it back to the
   stored server record.

2. The main "Connect to Server" flow (ServerNode.connect()) only
   persisted a freshly entered password to the server's stored
   ciphertext when the current request's save_password flag was
   true. But the password-prompt dialog shown on a failed connect
   doesn't resend the server's existing save_password setting -- it
   only reports its own checkbox state, which defaults to
   unchecked -- so a server already configured to save its password
   never got its stale ciphertext replaced. Separately, the driver's
   in-memory manager.password fix was never persisted via
   manager.update_session(); Driver.managers is an in-process cache,
   so in a multi-worker deployment (e.g. OpenShift/Helm) the next
   request can land on a different worker, which restores the stale
   pre-fix manager from the session and loses the corrected
   password.

Fix all three: persist the new password to the (owned or shared)
server record from the Query Tool reconnect path when save_password
is set and allowed; treat save_password as true in the main connect
flow whenever the server already has it enabled, not just when the
current request's flag says so; and call manager.update_session()
after a successful connect that used a freshly entered password.

Fixes #10128
---
 .../browser/server_groups/servers/__init__.py | 22 +++++++++-
 web/pgadmin/tools/sqleditor/__init__.py       | 41 +++++++++++++++++--
 2 files changed, 58 insertions(+), 5 deletions(-)

diff --git a/web/pgadmin/browser/server_groups/servers/__init__.py b/web/pgadmin/browser/server_groups/servers/__init__.py
index cfcb324c4d8..93bcc38a147 100644
--- a/web/pgadmin/browser/server_groups/servers/__init__.py
+++ b/web/pgadmin/browser/server_groups/servers/__init__.py
@@ -1685,8 +1685,16 @@ def connect(self, gid, sid, is_qt=False, server=None):
                 password = conn_passwd or server.password
         else:
             password = data['password'] if 'password' in data else None
-            save_password = data['save_password']\
-                if 'save_password' in data else False
+            # The password-prompt dialog doesn't resend the server's
+            # existing save_password setting, only its own checkbox
+            # state. If the server is already configured to save its
+            # password, a freshly entered replacement should still be
+            # persisted, or the stale saved password is never
+            # replaced. See issue #10128.
+            save_password = (
+                data['save_password'] if 'save_password' in data
+                else False
+            ) or bool(server.save_password)
 
             try:
                 # Encrypt the password before saving with user's login
@@ -1756,6 +1764,16 @@ def connect(self, gid, sid, is_qt=False, server=None):
 
                     return internal_server_error(errormsg=str(e))
 
+            # Persist the manager's corrected in-memory password (set by
+            # the driver on a successful connect) into the Flask session.
+            # Without this, a fresh worker process handling the next
+            # request (e.g. opening the Query Tool, in a multi-worker
+            # deployment) restores the stale pre-fix manager state from
+            # the session and the new password is lost, re-triggering the
+            # password prompt indefinitely. See issue #10128.
+            if password:
+                manager.update_session()
+
             if save_tunnel_password and config.ALLOW_SAVE_TUNNEL_PASSWORD:
                 try:
                     # Save the encrypted tunnel password.
diff --git a/web/pgadmin/tools/sqleditor/__init__.py b/web/pgadmin/tools/sqleditor/__init__.py
index 8080d220a54..960709d0baa 100644
--- a/web/pgadmin/tools/sqleditor/__init__.py
+++ b/web/pgadmin/tools/sqleditor/__init__.py
@@ -2713,7 +2713,7 @@ def connect_server(sid):
         # password the user just entered at that prompt is cached here so the
         # tool's connection can use it, instead of being discarded and
         # re-prompted in a loop.
-        _cache_manager_password_from_request(manager)
+        _cache_manager_password_from_request(manager, server)
         return make_json_response(
             success=1,
             info=gettext("Server connected."),
@@ -2726,7 +2726,7 @@ def connect_server(sid):
     )
 
 
-def _cache_manager_password_from_request(manager):
+def _cache_manager_password_from_request(manager, server=None):
     """
     Cache the password supplied with the current request (from a tool's
     password prompt) onto the server manager, so that connections opened by
@@ -2737,6 +2737,13 @@ def _cache_manager_password_from_request(manager):
     password, so a freshly entered credential (e.g. a regenerated, short-lived
     cloud auth token) takes effect immediately.
 
+    When "Save Password" is requested and allowed, the freshly entered
+    password is also persisted to the server record (overwriting any stale
+    stored ciphertext).  Without this, a rotated/regenerated password entered
+    at the prompt would work for the current session only and the tool would
+    keep re-using the stale saved password and re-prompt on the next
+    connection.
+
     This is best-effort: any failure (including malformed request data) is
     logged and swallowed so it never turns the caller's "Server connected"
     response into a 500 error.
@@ -2757,12 +2764,40 @@ def _cache_manager_password_from_request(manager):
         if not crypt_key_present:
             return
 
-        manager._update_password(encrypt(password, crypt_key))
+        enc_password = encrypt(password, crypt_key)
+        manager._update_password(enc_password)
         manager.update_session()
+
+        # Persist the freshly entered password if the user asked to save it,
+        # so the stale stored ciphertext is replaced.
+        save_password = data.get('save_password', False)
+        if save_password in ('true', 'True', '1', 1, True) and \
+                ALLOW_SAVE_PASSWORD and server is not None:
+            _persist_saved_password(server, enc_password)
     except Exception as e:
         current_app.logger.exception(e)
 
 
+def _persist_saved_password(server, enc_password):
+    """
+    Persist the encrypted password to the server record (owned or shared),
+    replacing any stale stored ciphertext.
+    """
+    from pgadmin.model import db
+    from pgadmin.browser.server_groups.servers import ServerModule
+
+    target = server
+    if server.shared and server.user_id != current_user.id:
+        shared_server = ServerModule.get_shared_server(
+            server, server.servergroup_id)
+        if shared_server is not None:
+            target = shared_server
+
+    setattr(target, 'save_password', 1)
+    setattr(target, 'password', enc_password)
+    db.session.commit()
+
+
 @blueprint.route(
     '/filter_dialog/<int:trans_id>',
     methods=["PUT"], endpoint='set_filter_data'

From 891b5c8a22932bb7511659a5e52147da0e3b8cb9 Mon Sep 17 00:00:00 2001
From: Kundan Sable <kundan.sable@enterprisedb.com>
Date: Mon, 7 Sep 2026 14:13:20 +0530
Subject: [PATCH 2/4] fix: address review feedback on saved-password
 persistence

- Move the manager.update_session() call for a corrected password into
  the driver's connect() (right after manager._update_password()) so
  every connect path benefits, not just ServerNode.connect. Drops the
  redundant/ineffective call that was added there.
- Query Tool's reconnect path never uses the freshly typed password to
  open a real connection, so validate it against the server (via a
  throwaway connection) before persisting it -- a typo at the prompt
  must not silently overwrite a working saved password.
- Seed the "Save Password" checkbox from the server's current setting
  and always send its state, so the backend can tell "explicitly
  unchecked" apart from "field not sent" and stops overriding an
  explicit uncheck.
- Roll back the session on a failed password-persist commit, reuse
  _is_non_owner() instead of a duplicate inline check, and consolidate
  boolean-ish request field parsing into pgadmin.utils.str_to_bool.
- Add unit tests for the owner/shared routing, rollback-on-failure, and
  password validation/boolean-parsing helpers.

Addresses review comments on PR #10178.
---
 .../browser/server_groups/servers/__init__.py |  32 ++---
 .../js/Dialogs/ConnectServerContent.jsx       |  11 +-
 web/pgadmin/tools/sqleditor/__init__.py       |  48 +++++--
 .../tests/test_persist_saved_password.py      | 125 ++++++++++++++++++
 web/pgadmin/utils/__init__.py                 |  10 ++
 .../utils/driver/psycopg3/connection.py       |   8 ++
 6 files changed, 202 insertions(+), 32 deletions(-)
 create mode 100644 web/pgadmin/tools/sqleditor/tests/test_persist_saved_password.py

diff --git a/web/pgadmin/browser/server_groups/servers/__init__.py b/web/pgadmin/browser/server_groups/servers/__init__.py
index 93bcc38a147..c52313829d6 100644
--- a/web/pgadmin/browser/server_groups/servers/__init__.py
+++ b/web/pgadmin/browser/server_groups/servers/__init__.py
@@ -44,7 +44,7 @@
 from sqlalchemy.orm.attributes import flag_modified
 from pgadmin.utils.preferences import Preferences
 from .... import socketio as sio
-from pgadmin.utils import get_complete_file_path
+from pgadmin.utils import get_complete_file_path, str_to_bool
 from pgadmin.settings.utils import with_object_filters
 from pgadmin.utils.server_access import get_server, \
     get_user_server_query, get_server_group
@@ -1685,16 +1685,14 @@ def connect(self, gid, sid, is_qt=False, server=None):
                 password = conn_passwd or server.password
         else:
             password = data['password'] if 'password' in data else None
-            # The password-prompt dialog doesn't resend the server's
-            # existing save_password setting, only its own checkbox
-            # state. If the server is already configured to save its
-            # password, a freshly entered replacement should still be
-            # persisted, or the stale saved password is never
-            # replaced. See issue #10128.
-            save_password = (
-                data['save_password'] if 'save_password' in data
-                else False
-            ) or bool(server.save_password)
+            # The password-prompt dialog seeds its checkbox from the
+            # server's current save_password setting (see
+            # get_response_for_password) and always sends its state, so
+            # this reflects the user's explicit choice -- including
+            # unchecking it for a server previously configured to save
+            # its password.
+            save_password = str_to_bool(
+                data['save_password'] if 'save_password' in data else False)
 
             try:
                 # Encrypt the password before saving with user's login
@@ -1764,16 +1762,6 @@ def connect(self, gid, sid, is_qt=False, server=None):
 
                     return internal_server_error(errormsg=str(e))
 
-            # Persist the manager's corrected in-memory password (set by
-            # the driver on a successful connect) into the Flask session.
-            # Without this, a fresh worker process handling the next
-            # request (e.g. opening the Query Tool, in a multi-worker
-            # deployment) restores the stale pre-fix manager state from
-            # the session and the new password is lost, re-triggering the
-            # password prompt indefinitely. See issue #10128.
-            if password:
-                manager.update_session()
-
             if save_tunnel_password and config.ALLOW_SAVE_TUNNEL_PASSWORD:
                 try:
                     # Save the encrypted tunnel password.
@@ -2213,6 +2201,7 @@ def get_response_for_password(self, server, status, prompt_password=False,
                 "service": server.service,
                 "prompt_tunnel_password": prompt_tunnel_password,
                 "prompt_password": prompt_password,
+                "save_password": bool(server.save_password),
                 "allow_save_password":
                     True if config.ALLOW_SAVE_PASSWORD and
                     'allow_save_password' in session and
@@ -2235,6 +2224,7 @@ def get_response_for_password(self, server, status, prompt_password=False,
                 "errmsg": errmsg,
                 "service": server.service,
                 "prompt_password": True,
+                "save_password": bool(server.save_password),
                 "allow_save_password":
                     True if config.ALLOW_SAVE_PASSWORD and
                     'allow_save_password' in session and
diff --git a/web/pgadmin/static/js/Dialogs/ConnectServerContent.jsx b/web/pgadmin/static/js/Dialogs/ConnectServerContent.jsx
index 796e1441c3a..0f07430b0b7 100644
--- a/web/pgadmin/static/js/Dialogs/ConnectServerContent.jsx
+++ b/web/pgadmin/static/js/Dialogs/ConnectServerContent.jsx
@@ -26,7 +26,10 @@ export default function ConnectServerContent({closeModal, data, onOK, setHeight,
     tunnel_password: '',
     save_tunnel_password: false,
     password: '',
-    save_password: false,
+    // Seed the checkbox from the server's current setting so that, for a
+    // server already configured to save its password, the checkbox
+    // reflects that instead of always defaulting to unchecked.
+    save_password: Boolean(data?.save_password),
   });
 
   const onTextChange = (e, id) => {
@@ -119,8 +122,10 @@ export default function ConnectServerContent({closeModal, data, onOK, setHeight,
             }
             if(data.prompt_password) {
               postFormData.append('password', formData.password);
-              formData.save_password &&
-                postFormData.append('save_password', formData.save_password);
+              // Always send the checkbox state (rather than only when
+              // checked) so the backend can tell "explicitly unchecked"
+              // apart from "field not sent".
+              postFormData.append('save_password', formData.save_password);
             }
             onOK?.(postFormData);
             closeModal();
diff --git a/web/pgadmin/tools/sqleditor/__init__.py b/web/pgadmin/tools/sqleditor/__init__.py
index 960709d0baa..6e70a4749d4 100644
--- a/web/pgadmin/tools/sqleditor/__init__.py
+++ b/web/pgadmin/tools/sqleditor/__init__.py
@@ -42,7 +42,7 @@
 from pgadmin.tools.sqleditor.utils.update_session_grid_transaction import \
     update_session_grid_transaction
 from pgadmin.utils import PgAdminModule
-from pgadmin.utils import get_storage_directory
+from pgadmin.utils import get_storage_directory, str_to_bool
 from pgadmin.utils.ajax import make_json_response, bad_request, \
     success_return, internal_server_error, service_unavailable, gone
 from pgadmin.utils.driver import get_driver
@@ -2769,25 +2769,53 @@ def _cache_manager_password_from_request(manager, server=None):
         manager.update_session()
 
         # Persist the freshly entered password if the user asked to save it,
-        # so the stale stored ciphertext is replaced.
-        save_password = data.get('save_password', False)
-        if save_password in ('true', 'True', '1', 1, True) and \
-                ALLOW_SAVE_PASSWORD and server is not None:
+        # so the stale stored ciphertext is replaced. This request never
+        # actually uses `password` to open a connection (the manager's
+        # primary connection was already established beforehand), so the
+        # password must be validated against the server first -- otherwise
+        # a typo at the prompt would silently overwrite a working saved
+        # password.
+        if str_to_bool(data.get('save_password', False)) and \
+                ALLOW_SAVE_PASSWORD and server is not None and \
+                _password_is_valid(manager, password):
             _persist_saved_password(server, enc_password)
     except Exception as e:
         current_app.logger.exception(e)
 
 
+def _password_is_valid(manager, password):
+    """
+    Verify that `password` (plaintext) actually authenticates against the
+    server, using a standalone connection that is closed immediately
+    afterwards -- it is never registered with the manager.
+    """
+    import psycopg
+    try:
+        conn_string = manager.create_connection_string(
+            manager.db, manager.user, password)
+        test_conn = psycopg.Connection.connect(
+            conn_string, connect_timeout=10)
+        test_conn.close()
+        return True
+    except psycopg.Error as e:
+        current_app.logger.info(
+            'Not persisting the re-entered password: it failed '
+            f'validation against the server.\nError: {e}'
+        )
+        return False
+
+
 def _persist_saved_password(server, enc_password):
     """
     Persist the encrypted password to the server record (owned or shared),
     replacing any stale stored ciphertext.
     """
     from pgadmin.model import db
-    from pgadmin.browser.server_groups.servers import ServerModule
+    from pgadmin.browser.server_groups.servers import (
+        ServerModule, _is_non_owner)
 
     target = server
-    if server.shared and server.user_id != current_user.id:
+    if _is_non_owner(server):
         shared_server = ServerModule.get_shared_server(
             server, server.servergroup_id)
         if shared_server is not None:
@@ -2795,7 +2823,11 @@ def _persist_saved_password(server, enc_password):
 
     setattr(target, 'save_password', 1)
     setattr(target, 'password', enc_password)
-    db.session.commit()
+    try:
+        db.session.commit()
+    except Exception:
+        db.session.rollback()
+        raise
 
 
 @blueprint.route(
diff --git a/web/pgadmin/tools/sqleditor/tests/test_persist_saved_password.py b/web/pgadmin/tools/sqleditor/tests/test_persist_saved_password.py
new file mode 100644
index 00000000000..ddec4bb284a
--- /dev/null
+++ b/web/pgadmin/tools/sqleditor/tests/test_persist_saved_password.py
@@ -0,0 +1,125 @@
+##########################################################################
+#
+# pgAdmin 4 - PostgreSQL Tools
+#
+# Copyright (C) 2013 - 2026, The pgAdmin Development Team
+# This software is released under the PostgreSQL Licence
+#
+##########################################################################
+
+"""
+Unit tests for the reconnect-path password persistence added for #10128.
+
+These exercise _persist_saved_password's owner/shared routing and
+_password_is_valid's pass/fail behaviour directly, with the DB and psycopg
+layers mocked out -- they don't need a live Postgres server connection.
+"""
+
+import unittest
+from unittest.mock import patch, MagicMock
+
+from pgadmin.utils.route import BaseTestGenerator
+from pgadmin.utils import str_to_bool
+from pgadmin.tools.sqleditor import _persist_saved_password, \
+    _password_is_valid
+
+
+class _NoServerSetupMixin:
+    """Skip BaseTestGenerator.setUp's Postgres connection -- these tests
+    exercise pure Python logic with mocked collaborators."""
+
+    def setUp(self):
+        unittest.TestCase.setUp(self)
+
+
+class TestPersistSavedPasswordOwner(_NoServerSetupMixin, BaseTestGenerator):
+    """An owned server's rotated password is written to the Server row
+    itself."""
+
+    def runTest(self):
+        server = MagicMock(shared=False, user_id=1)
+
+        with patch('pgadmin.model.db') as mock_db, \
+                patch(
+                    'pgadmin.browser.server_groups.servers.ServerModule'
+                ) as mock_mod:
+            _persist_saved_password(server, b'enc-pwd')
+
+            self.assertEqual(server.save_password, 1)
+            self.assertEqual(server.password, b'enc-pwd')
+            mock_mod.get_shared_server.assert_not_called()
+            mock_db.session.commit.assert_called_once()
+
+
+class TestPersistSavedPasswordSharedNonOwner(
+        _NoServerSetupMixin, BaseTestGenerator):
+    """A non-owner's rotated password lands on their SharedServer row and
+    leaves the owner's Server row untouched."""
+
+    def runTest(self):
+        owner_server = MagicMock(shared=True, user_id=1, servergroup_id=7)
+        shared_server = MagicMock()
+
+        with patch('pgadmin.model.db') as mock_db, \
+                patch(
+                    'pgadmin.browser.server_groups.servers.ServerModule'
+                ) as mock_mod, \
+                patch(
+                    'pgadmin.browser.server_groups.servers.current_user'
+                ) as mock_user:
+            mock_user.id = 2  # not the owner (user_id=1)
+            mock_mod.get_shared_server.return_value = shared_server
+
+            _persist_saved_password(owner_server, b'enc-pwd')
+
+            mock_mod.get_shared_server.assert_called_once_with(
+                owner_server, 7)
+            self.assertEqual(shared_server.save_password, 1)
+            self.assertEqual(shared_server.password, b'enc-pwd')
+            # The owner's own row must never be touched for a shared
+            # connection used by a non-owner.
+            self.assertNotEqual(owner_server.password, b'enc-pwd')
+            mock_db.session.commit.assert_called_once()
+
+
+class TestPersistSavedPasswordRollsBackOnFailure(
+        _NoServerSetupMixin, BaseTestGenerator):
+
+    def runTest(self):
+        server = MagicMock(shared=False, user_id=1)
+
+        with patch('pgadmin.model.db') as mock_db, \
+                patch('pgadmin.browser.server_groups.servers.ServerModule'):
+            mock_db.session.commit.side_effect = Exception('boom')
+
+            with self.assertRaises(Exception):
+                _persist_saved_password(server, b'enc-pwd')
+
+            mock_db.session.rollback.assert_called_once()
+
+
+class TestPasswordIsValid(_NoServerSetupMixin, BaseTestGenerator):
+
+    def runTest(self):
+        manager = MagicMock(db='postgres', user='enterprisedb')
+        manager.create_connection_string.return_value = 'dsn'
+
+        with patch('psycopg.Connection.connect') as mock_connect:
+            mock_connect.return_value = MagicMock()
+            self.assertTrue(_password_is_valid(manager, 'correct-horse'))
+
+            import psycopg
+            mock_connect.side_effect = psycopg.OperationalError(
+                'auth failed')
+            self.assertFalse(_password_is_valid(manager, 'wrong'))
+
+
+class TestStrToBool(_NoServerSetupMixin, BaseTestGenerator):
+    """save_password may arrive as a real bool, an int, or one of several
+    string spellings depending on the client (JSON body vs FormData)."""
+
+    def runTest(self):
+        for truthy in (True, 1, '1', 'true', 'True', 'on', 'yes'):
+            self.assertTrue(str_to_bool(truthy), msg=repr(truthy))
+        for falsy in (False, 0, '0', 'false', 'False', '', None):
+            self.assertFalse(str_to_bool(falsy), msg=repr(falsy))
diff --git a/web/pgadmin/utils/__init__.py b/web/pgadmin/utils/__init__.py
index 0a57d7e6c0f..524be380dd6 100644
--- a/web/pgadmin/utils/__init__.py
+++ b/web/pgadmin/utils/__init__.py
@@ -358,6 +358,16 @@ def does_utility_exist(file):
     return error_msg
 
 
+TRUTHY_STRING_VALUES = ('true', '1', 'on', 'yes')
+
+
+def str_to_bool(value):
+    """Normalise a boolean-ish value received from form/JSON request data
+    (which may arrive as a real bool, an int, or one of several string
+    spellings depending on the client) into an actual bool."""
+    return str(value).lower() in TRUTHY_STRING_VALUES
+
+
 def get_server(sid, only_owned=False):
     """Fetch a server by ID with access check.
 
diff --git a/web/pgadmin/utils/driver/psycopg3/connection.py b/web/pgadmin/utils/driver/psycopg3/connection.py
index d07a16cefcd..02e7b3cf2bd 100644
--- a/web/pgadmin/utils/driver/psycopg3/connection.py
+++ b/web/pgadmin/utils/driver/psycopg3/connection.py
@@ -421,6 +421,14 @@ async def connectdbserver():
 
         if status and is_update_password:
             manager._update_password(encpass)
+            # Persist the corrected in-memory password to the Flask
+            # session. Driver.managers is only an in-process cache, so
+            # without this a fresh worker process handling a later
+            # request (e.g. opening the Query Tool, in a multi-worker
+            # deployment) would restore the stale pre-fix manager from
+            # the session and lose the corrected password, re-triggering
+            # the password prompt indefinitely. See issue #10128.
+            manager.update_session()
         else:
             if not self.reconnecting and is_update_password:
                 self.wasConnected = False

From 093adb81456ef0bc54b25f4b29d796bc0ebafedb Mon Sep 17 00:00:00 2001
From: Kundan Sable <kundan.sable@enterprisedb.com>
Date: Mon, 7 Sep 2026 14:58:27 +0530
Subject: [PATCH 3/4] fix: address new CodeRabbit findings on saved-password
 persistence

- Include save_password in the Query Tool's own 428 password-prompt
  payloads (initialize_viewdata, _init_sqleditor) so the dialog seeds
  its checkbox correctly there too, not just from ServerNode.connect.
- Clear a server's stored save_password/password when the user
  explicitly submits save_password=false for a server that had one
  saved, instead of only ever handling the "save" case. A new
  save_password_provided flag keeps legacy callers that omit the field
  from having a saved credential wiped out.
- Validate the reconnect-path password before caching it on the
  manager/session at all, not just before persisting it to the DB --
  a typo must not replace a working in-memory/session password either.
- Fix pycodestyle E123 in the new test file by switching stacked
  `with` clauses to nested `with` blocks.
---
 .../browser/server_groups/servers/__init__.py | 28 +++++++++-
 web/pgadmin/tools/sqleditor/__init__.py       | 23 +++++---
 .../tests/test_persist_saved_password.py      | 56 +++++++++----------
 3 files changed, 67 insertions(+), 40 deletions(-)

diff --git a/web/pgadmin/browser/server_groups/servers/__init__.py b/web/pgadmin/browser/server_groups/servers/__init__.py
index c52313829d6..b4d2361e4c0 100644
--- a/web/pgadmin/browser/server_groups/servers/__init__.py
+++ b/web/pgadmin/browser/server_groups/servers/__init__.py
@@ -1618,6 +1618,12 @@ def connect(self, gid, sid, is_qt=False, server=None):
         passfile = None
         tunnel_password = None
         save_password = False
+        # Distinguishes "the caller explicitly said false" from "the
+        # caller didn't mention save_password at all" -- only the former
+        # should clear an existing saved credential (see the success
+        # branch below); legacy callers that omit the field must not have
+        # a saved password silently wiped out from under them.
+        save_password_provided = False
         save_tunnel_password = False
         prompt_password = False
         prompt_tunnel_password = False
@@ -1691,8 +1697,9 @@ def connect(self, gid, sid, is_qt=False, server=None):
             # this reflects the user's explicit choice -- including
             # unchecking it for a server previously configured to save
             # its password.
+            save_password_provided = 'save_password' in data
             save_password = str_to_bool(
-                data['save_password'] if 'save_password' in data else False)
+                data['save_password'] if save_password_provided else False)
 
             try:
                 # Encrypt the password before saving with user's login
@@ -1760,6 +1767,25 @@ def connect(self, gid, sid, is_qt=False, server=None):
                     manager.release(database=server.maintenance_db)
                     conn = None
 
+                    return internal_server_error(errormsg=str(e))
+            elif save_password_provided and not save_password and \
+                    server.save_password and config.ALLOW_SAVE_PASSWORD:
+                # The user explicitly unticked "Save Password" on a server
+                # that had one saved -- clear it instead of leaving the
+                # now-stale credential and flag in place.
+                try:
+                    if _is_non_owner(server):
+                        setattr(shared_server, 'save_password', 0)
+                        setattr(shared_server, 'password', None)
+                    else:
+                        setattr(server, 'save_password', 0)
+                        setattr(server, 'password', None)
+                    db.session.commit()
+                except Exception as e:
+                    current_app.logger.exception(e)
+                    manager.release(database=server.maintenance_db)
+                    conn = None
+
                     return internal_server_error(errormsg=str(e))
 
             if save_tunnel_password and config.ALLOW_SAVE_TUNNEL_PASSWORD:
diff --git a/web/pgadmin/tools/sqleditor/__init__.py b/web/pgadmin/tools/sqleditor/__init__.py
index 6e70a4749d4..66357b1dce7 100644
--- a/web/pgadmin/tools/sqleditor/__init__.py
+++ b/web/pgadmin/tools/sqleditor/__init__.py
@@ -267,6 +267,7 @@ def initialize_viewdata(trans_id, cmd_type, obj_type, sgid, sid, did, obj_id):
                         "username": user or server.username,
                         "errmsg": msg,
                         "prompt_password": True,
+                        "save_password": bool(server.save_password),
                         "allow_save_password": True
                         if ALLOW_SAVE_PASSWORD and
                         session.get('allow_save_password', None)
@@ -592,6 +593,7 @@ def _init_sqleditor(trans_id, connect, sgid, sid, did, dbname=None, **kwargs):
                             "username": user or server.username,
                             "errmsg": msg,
                             "prompt_password": True,
+                            "save_password": bool(server.save_password),
                             "allow_save_password": True
                             if ALLOW_SAVE_PASSWORD and
                             session.get('allow_save_password', None)
@@ -2764,20 +2766,23 @@ def _cache_manager_password_from_request(manager, server=None):
         if not crypt_key_present:
             return
 
+        # This request never actually uses `password` to open a connection
+        # (the manager's primary connection was already established
+        # beforehand), so it must be validated against the server before
+        # caching it on the manager or persisting it -- otherwise a typo at
+        # the prompt would silently replace a working password, for the
+        # current session as well as in durable storage.
+        if not _password_is_valid(manager, password):
+            return
+
         enc_password = encrypt(password, crypt_key)
         manager._update_password(enc_password)
         manager.update_session()
 
-        # Persist the freshly entered password if the user asked to save it,
-        # so the stale stored ciphertext is replaced. This request never
-        # actually uses `password` to open a connection (the manager's
-        # primary connection was already established beforehand), so the
-        # password must be validated against the server first -- otherwise
-        # a typo at the prompt would silently overwrite a working saved
-        # password.
+        # Persist the freshly entered password if the user asked to save
+        # it, so the stale stored ciphertext is replaced.
         if str_to_bool(data.get('save_password', False)) and \
-                ALLOW_SAVE_PASSWORD and server is not None and \
-                _password_is_valid(manager, password):
+                ALLOW_SAVE_PASSWORD and server is not None:
             _persist_saved_password(server, enc_password)
     except Exception as e:
         current_app.logger.exception(e)
diff --git a/web/pgadmin/tools/sqleditor/tests/test_persist_saved_password.py b/web/pgadmin/tools/sqleditor/tests/test_persist_saved_password.py
index ddec4bb284a..a951c76e4df 100644
--- a/web/pgadmin/tools/sqleditor/tests/test_persist_saved_password.py
+++ b/web/pgadmin/tools/sqleditor/tests/test_persist_saved_password.py
@@ -38,17 +38,16 @@ class TestPersistSavedPasswordOwner(_NoServerSetupMixin, BaseTestGenerator):
 
     def runTest(self):
         server = MagicMock(shared=False, user_id=1)
+        servers_mod = 'pgadmin.browser.server_groups.servers.ServerModule'
 
-        with patch('pgadmin.model.db') as mock_db, \
-                patch(
-                    'pgadmin.browser.server_groups.servers.ServerModule'
-                ) as mock_mod:
-            _persist_saved_password(server, b'enc-pwd')
+        with patch('pgadmin.model.db') as mock_db:
+            with patch(servers_mod) as mock_mod:
+                _persist_saved_password(server, b'enc-pwd')
 
-            self.assertEqual(server.save_password, 1)
-            self.assertEqual(server.password, b'enc-pwd')
-            mock_mod.get_shared_server.assert_not_called()
-            mock_db.session.commit.assert_called_once()
+                self.assertEqual(server.save_password, 1)
+                self.assertEqual(server.password, b'enc-pwd')
+                mock_mod.get_shared_server.assert_not_called()
+                mock_db.session.commit.assert_called_once()
 
 
 class TestPersistSavedPasswordSharedNonOwner(
@@ -59,27 +58,24 @@ class TestPersistSavedPasswordSharedNonOwner(
     def runTest(self):
         owner_server = MagicMock(shared=True, user_id=1, servergroup_id=7)
         shared_server = MagicMock()
-
-        with patch('pgadmin.model.db') as mock_db, \
-                patch(
-                    'pgadmin.browser.server_groups.servers.ServerModule'
-                ) as mock_mod, \
-                patch(
-                    'pgadmin.browser.server_groups.servers.current_user'
-                ) as mock_user:
-            mock_user.id = 2  # not the owner (user_id=1)
-            mock_mod.get_shared_server.return_value = shared_server
-
-            _persist_saved_password(owner_server, b'enc-pwd')
-
-            mock_mod.get_shared_server.assert_called_once_with(
-                owner_server, 7)
-            self.assertEqual(shared_server.save_password, 1)
-            self.assertEqual(shared_server.password, b'enc-pwd')
-            # The owner's own row must never be touched for a shared
-            # connection used by a non-owner.
-            self.assertNotEqual(owner_server.password, b'enc-pwd')
-            mock_db.session.commit.assert_called_once()
+        servers_mod = 'pgadmin.browser.server_groups.servers'
+
+        with patch('pgadmin.model.db') as mock_db:
+            with patch(f'{servers_mod}.ServerModule') as mock_mod:
+                with patch(f'{servers_mod}.current_user') as mock_user:
+                    mock_user.id = 2  # not the owner (user_id=1)
+                    mock_mod.get_shared_server.return_value = shared_server
+
+                    _persist_saved_password(owner_server, b'enc-pwd')
+
+                    mock_mod.get_shared_server.assert_called_once_with(
+                        owner_server, 7)
+                    self.assertEqual(shared_server.save_password, 1)
+                    self.assertEqual(shared_server.password, b'enc-pwd')
+                    # The owner's own row must never be touched for a
+                    # shared connection used by a non-owner.
+                    self.assertNotEqual(owner_server.password, b'enc-pwd')
+                    mock_db.session.commit.assert_called_once()
 
 
 class TestPersistSavedPasswordRollsBackOnFailure(

From de709188ed056342803e877a6da69713465beb85 Mon Sep 17 00:00:00 2001
From: Kundan Sable <kundan.sable@enterprisedb.com>
Date: Mon, 7 Sep 2026 15:16:12 +0530
Subject: [PATCH 4/4] fix: address remaining CodeRabbit findings on
 saved-password persistence

- Clear the saved password on the Query Tool reconnect path
  (_cache_manager_password_from_request) when save_password is
  explicitly false, mirroring the opt-out handling already present in
  ServerNode.connect.
- Keep the shared-server connect response's is_password_saved flag in
  sync with the SharedServer row for non-owners, instead of reading
  the stale detached overlay.
- Fix TestPasswordIsValid: _password_is_valid logs via current_app, so
  the test needs an application context to avoid a RuntimeError.
---
 .../browser/server_groups/servers/__init__.py |  9 +++
 web/pgadmin/tools/sqleditor/__init__.py       | 59 ++++++++++++++++---
 .../tests/test_persist_saved_password.py      |  3 +-
 3 files changed, 61 insertions(+), 10 deletions(-)

diff --git a/web/pgadmin/browser/server_groups/servers/__init__.py b/web/pgadmin/browser/server_groups/servers/__init__.py
index b4d2361e4c0..be61770778c 100644
--- a/web/pgadmin/browser/server_groups/servers/__init__.py
+++ b/web/pgadmin/browser/server_groups/servers/__init__.py
@@ -1750,6 +1750,12 @@ def connect(self, gid, sid, is_qt=False, server=None):
                     # 1 is True in SQLite as no boolean type
                     if _is_non_owner(server):
                         setattr(shared_server, 'save_password', 1)
+                        # `server` is a detached overlay (see
+                        # get_shared_server_properties) built before this
+                        # write, so it won't pick up the SharedServer
+                        # change on its own -- keep it in sync since the
+                        # connect response below reports its state.
+                        server.save_password = 1
                     else:
                         setattr(server, 'save_password', 1)
 
@@ -1777,6 +1783,9 @@ def connect(self, gid, sid, is_qt=False, server=None):
                     if _is_non_owner(server):
                         setattr(shared_server, 'save_password', 0)
                         setattr(shared_server, 'password', None)
+                        # Keep the detached overlay in sync -- see the
+                        # comment in the save_password branch above.
+                        server.save_password = 0
                     else:
                         setattr(server, 'save_password', 0)
                         setattr(server, 'password', None)
diff --git a/web/pgadmin/tools/sqleditor/__init__.py b/web/pgadmin/tools/sqleditor/__init__.py
index 66357b1dce7..2511d8c7db4 100644
--- a/web/pgadmin/tools/sqleditor/__init__.py
+++ b/web/pgadmin/tools/sqleditor/__init__.py
@@ -2779,11 +2779,22 @@ def _cache_manager_password_from_request(manager, server=None):
         manager._update_password(enc_password)
         manager.update_session()
 
+        if server is None or not ALLOW_SAVE_PASSWORD:
+            return
+
+        save_password_provided = 'save_password' in data
+        save_password = str_to_bool(data.get('save_password', False))
+
         # Persist the freshly entered password if the user asked to save
-        # it, so the stale stored ciphertext is replaced.
-        if str_to_bool(data.get('save_password', False)) and \
-                ALLOW_SAVE_PASSWORD and server is not None:
+        # it, so the stale stored ciphertext is replaced. An explicit
+        # false instead clears any previously saved credential -- mirrors
+        # the same "Save Password" opt-out handling in
+        # browser.server_groups.servers.ServerNode.connect -- so
+        # unchecking the box here doesn't leave a stale saved password.
+        if save_password:
             _persist_saved_password(server, enc_password)
+        elif save_password_provided:
+            _clear_saved_password(server)
     except Exception as e:
         current_app.logger.exception(e)
 
@@ -2810,22 +2821,31 @@ def _password_is_valid(manager, password):
         return False
 
 
-def _persist_saved_password(server, enc_password):
+def _get_save_password_target(server):
     """
-    Persist the encrypted password to the server record (owned or shared),
-    replacing any stale stored ciphertext.
+    Return the record ("save_password"/"password" live on the owned Server
+    row, or on the current user's SharedServer row for a shared server they
+    don't own).
     """
-    from pgadmin.model import db
     from pgadmin.browser.server_groups.servers import (
         ServerModule, _is_non_owner)
 
-    target = server
     if _is_non_owner(server):
         shared_server = ServerModule.get_shared_server(
             server, server.servergroup_id)
         if shared_server is not None:
-            target = shared_server
+            return shared_server
+    return server
 
+
+def _persist_saved_password(server, enc_password):
+    """
+    Persist the encrypted password to the server record (owned or shared),
+    replacing any stale stored ciphertext.
+    """
+    from pgadmin.model import db
+
+    target = _get_save_password_target(server)
     setattr(target, 'save_password', 1)
     setattr(target, 'password', enc_password)
     try:
@@ -2835,6 +2855,27 @@ def _persist_saved_password(server, enc_password):
         raise
 
 
+def _clear_saved_password(server):
+    """
+    Clear a previously saved password on the owned or shared server record,
+    so an explicit "Save Password" opt-out doesn't leave a stale saved
+    credential behind.
+    """
+    from pgadmin.model import db
+
+    target = _get_save_password_target(server)
+    if not target.save_password:
+        return
+
+    setattr(target, 'save_password', 0)
+    setattr(target, 'password', None)
+    try:
+        db.session.commit()
+    except Exception:
+        db.session.rollback()
+        raise
+
+
 @blueprint.route(
     '/filter_dialog/<int:trans_id>',
     methods=["PUT"], endpoint='set_filter_data'
diff --git a/web/pgadmin/tools/sqleditor/tests/test_persist_saved_password.py b/web/pgadmin/tools/sqleditor/tests/test_persist_saved_password.py
index a951c76e4df..875ee684902 100644
--- a/web/pgadmin/tools/sqleditor/tests/test_persist_saved_password.py
+++ b/web/pgadmin/tools/sqleditor/tests/test_persist_saved_password.py
@@ -100,7 +100,8 @@ def runTest(self):
         manager = MagicMock(db='postgres', user='enterprisedb')
         manager.create_connection_string.return_value = 'dsn'
 
-        with patch('psycopg.Connection.connect') as mock_connect:
+        with self.app.app_context(), \
+                patch('psycopg.Connection.connect') as mock_connect:
             mock_connect.return_value = MagicMock()
             self.assertTrue(_password_is_valid(manager, 'correct-horse'))