pgadmin-org
/
pgadmin4
mirror of https://github.com/pgadmin-org/pgadmin4.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

fix: SharedServer feature parity columns and write guards (#9835) 

Add passexec_cmd, passexec_expiration, kerberos_conn, tags, and
post_connection_sql to SharedServer so non-owners get their own
per-user values instead of inheriting the owner's.  Drop the unused
db_res column which was never overlaid or writable by non-owners.

Key changes:
- New Alembic migration (sharedserver_feature_parity) adds 5 columns,
  drops db_res, cleans up orphaned records.  All operations idempotent.
- Overlay copies new fields from SharedServer instead of suppressing
- _owner_only_fields guard blocks non-owners from setting passexec_cmd,
  passexec_expiration, db_res, db_res_type via API
- Non-owners can set post_connection_sql (runs under their own creds)
- update_tags and flag_modified use sharedserver for non-owners
- update() response returns sharedserver tags for non-owners
- ServerManager passexec suppression with config.SERVER_MODE guard
- UI: post_connection_sql editable for non-owners (readonly only when
  connected, not when shared)
- SCHEMA_VERSION bumped to 51
- Comprehensive unit tests for overlay, write guards, and tag deltas
dependabot/npm_and_yarn/runtime/globals-17.5.0
Ashesh Vashi 2026-04-13 15:03:31 +05:30 committed by GitHub
parent 4ddb16f47a
commit e4edcf2253
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 360 additions and 41 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

87
web/migrations/versions/sharedserver_feature_parity_.py Normal file
View File

@ -0,0 +1,87 @@
##########################################################################
#
# pgAdmin 4 - PostgreSQL Tools
#
# Copyright (C) 2013 - 2026, The pgAdmin Development Team
# This software is released under the PostgreSQL Licence
#
##########################################################################
"""SharedServer feature parity columns, orphan cleanup, and db_res removal
Adds passexec_cmd, passexec_expiration, kerberos_conn, tags, and
post_connection_sql to the sharedserver table so non-owners get their
own per-user values instead of inheriting the owner's. Drops db_res
which was never overlaid or writable by non-owners. Also cleans up
orphaned records whose parent server was deleted.
Revision ID: sharedserver_feature_parity
Revises: add_user_id_dbg_args
Create Date: 2026-04-13
"""
from alembic import op
import sqlalchemy as sa
# revision identifiers, used by Alembic.
revision = 'sharedserver_feature_parity'
down_revision = 'add_user_id_dbg_args'
branch_labels = None
depends_on = None
def upgrade():
conn = op.get_bind()
inspector = sa.inspect(conn)
if not inspector.has_table('sharedserver'):
return
# Clean up orphaned SharedServer records whose osid
# references a Server that no longer exists.
op.execute(
"DELETE FROM sharedserver WHERE osid NOT IN "
"(SELECT id FROM server)"
)
# Add missing columns (idempotent — guard against re-runs).
existing_cols = {
c['name'] for c in inspector.get_columns('sharedserver')
}
new_columns = [
('passexec_cmd',
sa.Column('passexec_cmd', sa.Text(),
nullable=True)),
('passexec_expiration',
sa.Column('passexec_expiration', sa.Integer(),
nullable=True)),
('kerberos_conn',
sa.Column('kerberos_conn', sa.Boolean(),
nullable=False,
server_default=sa.false())),
('tags',
sa.Column('tags', sa.JSON(), nullable=True)),
('post_connection_sql',
sa.Column('post_connection_sql', sa.String(),
nullable=True)),
]
cols_to_add = [
col for name, col in new_columns
if name not in existing_cols
]
if cols_to_add:
with op.batch_alter_table('sharedserver') as batch_op:
for col in cols_to_add:
batch_op.add_column(col)
# Drop db_res — database restrictions are an owner-level
# concept; the column was never overlaid or writable by
# non-owners and its presence invites accidental leakage.
if 'db_res' in existing_cols:
with op.batch_alter_table('sharedserver') as batch_op:
batch_op.drop_column('db_res')
def downgrade():
# pgAdmin only upgrades, downgrade not implemented.
pass

67
web/pgadmin/browser/server_groups/servers/__init__.py
View File

@ -172,9 +172,7 @@ class ServerModule(sg.ServerGroupPluginModule):
Return shared server properties.
Overlays per-user SharedServer values onto the owner's Server
object. Security-sensitive fields that are absent from the
SharedServer model (passexec_cmd, post_connection_sql) are
suppressed for non-owners.
object so each non-owner sees their own customizations.
The server is expunged from the SQLAlchemy session before
mutation so that the owner's record is never dirtied.
@ -182,6 +180,9 @@ class ServerModule(sg.ServerGroupPluginModule):
:param sharedserver:
:return: shared server (detached)
"""
if sharedserver is None:
return server
# Detach from session so in-place mutations are never
# flushed back to the owner's Server row.
sess = object_session(server)
@ -224,13 +225,11 @@ class ServerModule(sg.ServerGroupPluginModule):
server.server_owner = sharedserver.server_owner
server.password = sharedserver.password
server.prepare_threshold = sharedserver.prepare_threshold
# Suppress owner-only fields that are absent from SharedServer
# and dangerous when inherited (privilege escalation / code
# execution).
server.passexec_cmd = None
server.passexec_expiration = None
server.post_connection_sql = None
server.passexec_cmd = sharedserver.passexec_cmd
server.passexec_expiration = sharedserver.passexec_expiration
server.kerberos_conn = sharedserver.kerberos_conn
server.tags = sharedserver.tags
server.post_connection_sql = sharedserver.post_connection_sql
return server
@ -477,7 +476,12 @@ class ServerModule(sg.ServerGroupPluginModule):
tunnel_prompt_password=0,
shared=True,
connection_params=safe_conn_params,
prepare_threshold=data.prepare_threshold
prepare_threshold=data.prepare_threshold,
passexec_cmd=None,
passexec_expiration=None,
kerberos_conn=False,
tags=None,
post_connection_sql=None
)
db.session.add(shared_server)
db.session.commit()
@ -904,7 +908,7 @@ class ServerNode(PGChildNodeView):
# Update connection parameter if any.
self.update_connection_parameter(data, server, sharedserver)
self.update_tags(data, server)
self.update_tags(data, sharedserver or server)
if 'connection_params' in data and \
'hostaddr' in data['connection_params'] and \
@ -937,7 +941,7 @@ class ServerNode(PGChildNodeView):
# tags is JSON type, sqlalchemy sometimes will not detect change
if 'tags' in data:
flag_modified(server, 'tags')
flag_modified(sharedserver or server, 'tags')
try:
db.session.commit()
@ -953,6 +957,10 @@ class ServerNode(PGChildNodeView):
# which will affect the connections.
if not conn.connected():
manager.update(server)
# Suppress passexec for non-owners so the manager
# never holds the owner's password-exec command.
if _is_non_owner(server):
manager.passexec = None
return jsonify(
node=self.blueprint.generate_browser_node(
@ -974,7 +982,7 @@ class ServerNode(PGChildNodeView):
role=server.role,
is_password_saved=bool(server.save_password),
description=server.comment,
tags=server.tags
tags=(sharedserver or server).tags
)
)
@ -998,8 +1006,20 @@ class ServerNode(PGChildNodeView):
if not crypt_key_present:
raise CryptKeyMissing
# Fields that non-owners must never set on their
# SharedServer — they enable command/SQL execution
# or are owner-level concepts not on SharedServer.
_owner_only_fields = frozenset({
'passexec_cmd', 'passexec_expiration',
'db_res', 'db_res_type',
})
for arg in config_param_map:
if arg in data:
# Non-owners cannot set dangerous fields.
if _is_non_owner(server) and \
arg in _owner_only_fields:
continue
value = data[arg]
if arg == 'password':
value = encrypt(data[arg], crypt_key)
@ -1159,14 +1179,8 @@ class ServerNode(PGChildNodeView):
'fgcolor': server.fgcolor,
'db_res': get_db_restriction(server.db_res_type, server.db_res),
'db_res_type': server.db_res_type,
'passexec_cmd':
server.passexec_cmd
if server.passexec_cmd and
not _is_non_owner(server) else None,
'passexec_expiration':
server.passexec_expiration
if server.passexec_expiration and
not _is_non_owner(server) else None,
'passexec_cmd': server.passexec_cmd,
'passexec_expiration': server.passexec_expiration,
'service': server.service if server.service else None,
'use_ssh_tunnel': use_ssh_tunnel,
'tunnel_host': tunnel_host,
@ -1186,8 +1200,7 @@ class ServerNode(PGChildNodeView):
'connection_string': display_connection_str,
'prepare_threshold': server.prepare_threshold,
'tags': tags,
'post_connection_sql': server.post_connection_sql
if not _is_non_owner(server) else None,
'post_connection_sql': server.post_connection_sql,
}
return ajax_response(response)
@ -1605,6 +1618,12 @@ class ServerNode(PGChildNodeView):
# the API call is not made from SQL Editor or View/Edit Data tool
if not manager.connection().connected() and not is_qt:
manager.update(server)
# Re-suppress passexec after update() which rebuilds
# from the (overlaid) server object. Belt-and-suspenders:
# the overlay already defaults passexec to None, but this
# guards against direct DB edits.
if _is_non_owner(server):
manager.passexec = None
conn = manager.connection()
# Get enc key

2
web/pgadmin/browser/server_groups/servers/static/js/server.ui.js
View File

@ -587,7 +587,7 @@ export default class ServerSchema extends BaseUISchema {
group: gettext('Post Connection SQL'),
mode: ['properties', 'edit', 'create'],
type: 'sql', isFullTab: true,
readonly: obj.isConnectedOrShared,
readonly: obj.isConnected,
helpMessage: gettext('Any query specified in the control below will be executed with autocommit mode enabled for each connection to any database on this server.'),
},
{

220
web/pgadmin/browser/server_groups/servers/tests/test_shared_server_unit.py
View File

@ -84,6 +84,11 @@ def _make_shared_server(**overrides):
'sslcert': '/home/nonowner/.ssl/cert.pem',
'connect_timeout': '10',
},
passexec_cmd=None,
passexec_expiration=None,
kerberos_conn=False,
tags=None,
post_connection_sql=None,
)
defaults.update(overrides)
ss = MagicMock()
@ -97,10 +102,12 @@ class TestGetSharedServerProperties(BaseTestGenerator):
using mock objects."""
scenarios = [
('Merge suppresses passexec_cmd',
dict(test_method='test_suppresses_passexec')),
('Merge suppresses post_connection_sql',
dict(test_method='test_suppresses_post_sql')),
('Merge overlays passexec_cmd from SharedServer',
dict(test_method='test_overlays_passexec')),
('Merge overlays post_connection_sql from SharedServer',
dict(test_method='test_overlays_post_sql')),
('Merge overlays kerberos_conn and tags',
dict(test_method='test_overlays_kerberos_tags')),
('Merge strips owner SSL paths not in SharedServer',
dict(test_method='test_strips_owner_ssl_paths')),
('Merge applies SharedServer SSL paths',
@ -111,6 +118,8 @@ class TestGetSharedServerProperties(BaseTestGenerator):
dict(test_method='test_overrides_tunnel')),
('Merge handles None connection_params',
dict(test_method='test_none_conn_params')),
('Merge returns server unchanged when sharedserver is None',
dict(test_method='test_null_guard')),
]
@patch('pgadmin.browser.server_groups.servers.'
@ -128,14 +137,41 @@ class TestGetSharedServerProperties(BaseTestGenerator):
return ServerModule.get_shared_server_properties(
server, ss)
def test_suppresses_passexec(self):
def test_overlays_passexec(self):
# SharedServer defaults have None - overlay copies that.
result = self._merge()
self.assertIsNone(result.passexec_cmd)
self.assertIsNone(result.passexec_expiration)
# If SharedServer has a value, it should appear.
ss = _make_shared_server(
passexec_cmd='/usr/bin/get-pw',
passexec_expiration=120)
result = self._merge(ss=ss)
self.assertEqual(result.passexec_cmd, '/usr/bin/get-pw')
self.assertEqual(result.passexec_expiration, 120)
def test_suppresses_post_sql(self):
def test_overlays_post_sql(self):
# SharedServer defaults have None - overlay copies that.
result = self._merge()
self.assertIsNone(result.post_connection_sql)
# If SharedServer has a value, it should appear.
ss = _make_shared_server(
post_connection_sql='SET role reader;')
result = self._merge(ss=ss)
self.assertEqual(
result.post_connection_sql, 'SET role reader;')
def test_overlays_kerberos_tags(self):
result = self._merge()
self.assertFalse(result.kerberos_conn)
self.assertIsNone(result.tags)
# With values set on SharedServer
ss = _make_shared_server(
kerberos_conn=True,
tags=[{'text': 'prod', 'color': '#f00'}])
result = self._merge(ss=ss)
self.assertTrue(result.kerberos_conn)
self.assertEqual(len(result.tags), 1)
def test_strips_owner_ssl_paths(self):
result = self._merge()
@ -180,6 +216,18 @@ class TestGetSharedServerProperties(BaseTestGenerator):
# Should not crash; connection_params becomes {}
self.assertEqual(result.connection_params, {})
def test_null_guard(self):
from pgadmin.browser.server_groups.servers import \
ServerModule
server = _make_server()
# Call directly to bypass _merge's None replacement
result = ServerModule.get_shared_server_properties(
server, None)
# Should return server unchanged
self.assertEqual(result.name, 'OwnerServer')
self.assertEqual(result.passexec_cmd,
'/usr/bin/vault-get-secret')
class TestCreateSharedServerSanitization(BaseTestGenerator):
"""Verify create_shared_server() strips sensitive
@ -291,6 +339,7 @@ class TestMergeExpungesServer(BaseTestGenerator):
# Should not crash
result = ServerModule.get_shared_server_properties(
server, ss)
# SharedServer defaults passexec_cmd to None
self.assertIsNone(result.passexec_cmd)
@ -457,6 +506,165 @@ class TestDeleteSharedServerOwnerGuard(BaseTestGenerator):
1, server.id)
class TestOwnerOnlyFieldsGuard(BaseTestGenerator):
"""Verify _set_valid_attr_value skips owner-only fields
for non-owners."""
scenarios = [
('Non-owner cannot set passexec_cmd',
dict(test_method='test_nonowner_passexec_blocked')),
('Non-owner cannot set db_res or db_res_type',
dict(test_method='test_nonowner_db_res_blocked')),
('Owner can set passexec_cmd',
dict(test_method='test_owner_passexec_allowed')),
]
def runTest(self):
getattr(self, self.test_method)()
@patch(SRV_MODULE + '.get_crypt_key',
return_value=(True, b'key'))
@patch(SRV_MODULE + '.current_user')
def test_nonowner_passexec_blocked(self, mock_cu, mock_ck):
mock_cu.id = 200 # Non-owner
from pgadmin.browser.server_groups.servers import \
ServerNode
server = _make_server()
ss = _make_shared_server()
node = ServerNode.__new__(ServerNode)
node.delete_shared_server = MagicMock()
data = {
'passexec_cmd': '/evil/cmd',
'post_connection_sql': 'SET role reader;',
}
config_map = {
'passexec_cmd': 'passexec_cmd',
'post_connection_sql': 'post_connection_sql',
}
node._set_valid_attr_value(
1, data, config_map, server, ss)
# passexec_cmd should be blocked for non-owners
self.assertIsNone(ss.passexec_cmd)
# post_connection_sql is allowed for non-owners
self.assertEqual(ss.post_connection_sql,
'SET role reader;')
@patch(SRV_MODULE + '.get_crypt_key',
return_value=(True, b'key'))
@patch(SRV_MODULE + '.current_user')
def test_nonowner_db_res_blocked(self, mock_cu, mock_ck):
mock_cu.id = 200 # Non-owner
from pgadmin.browser.server_groups.servers import \
ServerNode
server = _make_server()
ss = _make_shared_server()
node = ServerNode.__new__(ServerNode)
node.delete_shared_server = MagicMock()
data = {
'db_res': 'secret_db',
'db_res_type': 'databases',
}
config_map = {
'db_res': 'db_res',
'db_res_type': 'db_res_type',
}
node._set_valid_attr_value(
1, data, config_map, server, ss)
# Neither server nor sharedserver should be modified
self.assertIsNone(server.db_res)
self.assertIsNone(server.db_res_type)
@patch(SRV_MODULE + '.get_crypt_key',
return_value=(True, b'key'))
@patch(SRV_MODULE + '.current_user')
def test_owner_passexec_allowed(self, mock_cu, mock_ck):
mock_cu.id = 100 # Owner
from pgadmin.browser.server_groups.servers import \
ServerNode
server = _make_server()
node = ServerNode.__new__(ServerNode)
node.delete_shared_server = MagicMock()
data = {
'passexec_cmd': '/usr/bin/new-cmd',
'post_connection_sql': 'SET role dba;',
}
config_map = {
'passexec_cmd': 'passexec_cmd',
'post_connection_sql': 'post_connection_sql',
}
node._set_valid_attr_value(
1, data, config_map, server, None)
# Owner should have these set
self.assertEqual(server.passexec_cmd, '/usr/bin/new-cmd')
self.assertEqual(
server.post_connection_sql, 'SET role dba;')
class TestUpdateTagsBase(BaseTestGenerator):
"""Verify update_tags reads from the correct base object."""
scenarios = [
('Non-owner tag delta uses sharedserver tags',
dict(test_method='test_nonowner_tags_base')),
('Owner tag delta uses server tags',
dict(test_method='test_owner_tags_base')),
]
def runTest(self):
getattr(self, self.test_method)()
def test_nonowner_tags_base(self):
from pgadmin.browser.server_groups.servers import \
ServerNode
server = _make_server(
tags=[{'text': 'owner-tag', 'color': '#000'}])
ss = _make_shared_server(
tags=[{'text': 'my-tag', 'color': '#f00'}])
data = {'tags': {
'deleted': [{'old_text': 'my-tag'}],
}}
# Non-owner: should delete from sharedserver's tags,
# not from server's (owner's) tags.
ServerNode.update_tags(data, ss)
# sharedserver had 'my-tag' which was deleted → empty
self.assertEqual(data['tags'], [])
def test_owner_tags_base(self):
from pgadmin.browser.server_groups.servers import \
ServerNode
server = _make_server(
tags=[{'text': 'owner-tag', 'color': '#000'}])
data = {'tags': {
'added': [{'text': 'new-tag', 'color': '#0f0'}],
}}
ServerNode.update_tags(data, server)
# owner had 'owner-tag', added 'new-tag' → 2 tags
self.assertEqual(len(data['tags']), 2)
texts = {t['text'] for t in data['tags']}
self.assertIn('owner-tag', texts)
self.assertIn('new-tag', texts)
class TestGetSharedServerRaisesOnNone(BaseTestGenerator):
"""Verify get_shared_server() raises if SharedServer
cannot be created."""

10
web/pgadmin/model/__init__.py
View File

@ -33,7 +33,7 @@ import config
#
##########################################################################
SCHEMA_VERSION = 50
SCHEMA_VERSION = 51
##########################################################################
#
@ -528,7 +528,6 @@ class SharedServer(db.Model, UserScopedMixin):
backref=db.backref('sharedserver', cascade=CASCADE_STR),
lazy='joined'
)
db_res = db.Column(db.Text(), nullable=True)
bgcolor = db.Column(db.String(10), nullable=True)
fgcolor = db.Column(db.String(10), nullable=True)
service = db.Column(db.Text(), nullable=True)
@ -560,6 +559,13 @@ class SharedServer(db.Model, UserScopedMixin):
shared = db.Column(db.Boolean(), nullable=False)
connection_params = db.Column(MutableDict.as_mutable(types.JSON))
prepare_threshold = db.Column(db.Integer(), nullable=True)
passexec_cmd = db.Column(db.Text(), nullable=True)
passexec_expiration = db.Column(db.Integer(), nullable=True)
kerberos_conn = db.Column(
db.Boolean(), nullable=False, default=False
)
tags = db.Column(types.JSON)
post_connection_sql = db.Column(db.String(), nullable=True)
class Macros(db.Model):

15
web/pgadmin/utils/driver/psycopg3/__init__.py
View File

@ -84,13 +84,12 @@ class Driver(BaseDriver):
for server in servers:
manager = managers[str(server.id)] = \
ServerManager(server)
# Suppress owner-only fields for non-owners
# of shared servers so passexec_cmd and
# post_connection_sql don't leak.
if server.shared and \
# Suppress passexec for non-owners of shared
# servers — it runs commands on the client
# machine and must not inherit the owner's.
if config.SERVER_MODE and server.shared and \
server.user_id != current_user.id:
manager.passexec = None
manager.post_connection_sql = None
if server.id in session_managers:
manager._restore(
session_managers[server.id])
@ -153,12 +152,12 @@ class Driver(BaseDriver):
# server_data was already access-checked above;
# it cannot be None at this point.
manager = ServerManager(server_data)
# Suppress owner-only fields for non-owners of
# shared servers.
# Suppress passexec for non-owners of shared
# servers — it runs commands on the client machine
# and must not inherit the owner's.
if config.SERVER_MODE and server_data.shared and \
server_data.user_id != current_user.id:
manager.passexec = None
manager.post_connection_sql = None
managers[str(sid)] = manager
return manager