From a868adbcf4935523fa3d24960a73307daf6c2cb5 Mon Sep 17 00:00:00 2001 From: Dave Page Date: Tue, 9 Jun 2026 12:05:08 +0100 Subject: [PATCH] Validate preference values set via the CLI. #7346 The CLI set-prefs path (save_pref -> Preferences.save_cli) wrote the raw value to the configuration database without any type validation and always reported success, unlike the GUI path which validates via _Preference.set(). Route save_cli through the same set() validation (set() now accepts an explicit user_id so it works outside a request context), and make setup.py set-prefs check the result and report preferences whose value was invalid. Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/en_US/release_notes_9_16.rst | 2 + web/pgadmin/utils/preferences.py | 65 ++++++++++++++++++++++--------- web/setup.py | 16 ++++++-- 3 files changed, 60 insertions(+), 23 deletions(-) diff --git a/docs/en_US/release_notes_9_16.rst b/docs/en_US/release_notes_9_16.rst index a7ec92e1bee..253d7c97997 100644 --- a/docs/en_US/release_notes_9_16.rst +++ b/docs/en_US/release_notes_9_16.rst @@ -25,3 +25,5 @@ Housekeeping Bug fixes ********* + + | `Issue #7346 `_ - Fixed an issue where preferences set via the CLI (setup.py set-prefs) were not validated, so invalid values were stored silently; CLI preference values are now validated against the preference type and rejected (and reported) if invalid. diff --git a/web/pgadmin/utils/preferences.py b/web/pgadmin/utils/preferences.py index 06ea7ab550f..c27d278f65a 100644 --- a/web/pgadmin/utils/preferences.py +++ b/web/pgadmin/utils/preferences.py @@ -165,12 +165,15 @@ def _get_format_data(self, res): return False, None - def set(self, value): + def set(self, value, user_id=None): """ set - Set the value into the configuration table for this current user. + Set the value into the configuration table for this current user + (or the given user_id, when called outside a request context). :param value: Value to be set + :param user_id: User to set the preference for; defaults to the + current user. :returns: nothing. """ @@ -217,14 +220,15 @@ def set(self, value): "Invalid value for {0} option.".format( error_map.get(self._type, self._type))) + uid = user_id if user_id is not None else current_user.id pref = UserPrefTable.query.filter_by( pid=self.pid - ).filter_by(uid=current_user.id).first() + ).filter_by(uid=uid).first() value = "{}".format(value) if pref is None: pref = UserPrefTable( - uid=current_user.id, pid=self.pid, value=value + uid=uid, pid=self.pid, value=value ) db.session.add(pref) else: @@ -597,30 +601,53 @@ def module(cls, name, create=True): @classmethod def save_cli(cls, mid, cid, pid, user_id, value): """ - save - Update the value for the preference in the configuration database. + save_cli + Validate and update the value for the preference in the + configuration database for the given user (used by the CLI). :param mid: Module ID :param cid: Category ID :param pid: Preference ID + :param user_id: User to set the preference for :param value: Value for the options """ + # Find the entry for this module in the configuration database. + module = ModulePrefTable.query.filter_by(id=mid).first() - pref = UserPrefTable.query.filter_by( - pid=pid - ).filter_by(uid=user_id).first() + if module is None: + return False, gettext("Could not find the specified module.") - value = "{}".format(value) - if pref is None: - pref = UserPrefTable( - uid=user_id, pid=pid, value=value - ) - db.session.add(pref) - else: - pref.value = value - db.session.commit() + m = cls.modules.get(module.name) + if m is None: + return False, gettext( + "Module '{0}' is no longer in use." + ).format(module.name) - return True, None + category = None + for c in m.categories: + cat = m.categories[c] + if cid == cat['id']: + category = cat + break + + if category is None: + return False, gettext( + "Module '{0}' does not have category with id '{1}'" + ).format(module.name, cid) + + preference = None + for p in category['preferences']: + pref = (category['preferences'])[p] + if pref.pid == pid: + preference = pref + break + + if preference is None: + return False, gettext("Could not find the specified preference.") + + # Delegate to set() so the value is validated against the + # preference type, just like the GUI path. + return preference.set(value, user_id=user_id) @classmethod def save(cls, mid, cid, pid, value): diff --git a/web/setup.py b/web/setup.py index 99dfc9ae78b..cdc3e6d0c26 100644 --- a/web/setup.py +++ b/web/setup.py @@ -728,6 +728,7 @@ def set_prefs(username, prefs = ManagePreferences.fetch_prefs(True) app = create_app(config.APP_NAME + '-cli') invalid_prefs = [] + invalid_value_prefs = [] valid_prefs = [] with app.app_context(): from pgadmin.preferences import save_pref @@ -749,11 +750,13 @@ def set_prefs(username, 'name': final_opt[2], 'user_id': user_id, 'value': val} - save_pref(_row) - valid_prefs.append(_row) + if save_pref(_row): + valid_prefs.append(_row) - if not json: - table.add_row(jsonlib.dumps(_row)) + if not json: + table.add_row(jsonlib.dumps(_row)) + else: + invalid_value_prefs.append(f) else: invalid_prefs.append(f) @@ -762,6 +765,11 @@ def set_prefs(username, (', ').join( invalid_prefs))) + if len(invalid_value_prefs) >= 1: + print("Invalid value provided for preference(s) " + "[red]{0}[/red].".format( + (', ').join(invalid_value_prefs))) + if not json and console: print(table) elif json and console: