Skip to content

fix: enforce data isolation and harden shared servers in server mode #9830

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
asheshv merged 3 commits into from
Apr 9, 2026
Merged

fix: enforce data isolation and harden shared servers in server mode #9830

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
25064f5
fix: enforce data isolation and harden shared servers in server mode
asheshv Apr 9, 2026
801d287
test: data isolation and shared server security tests
asheshv Apr 9, 2026
08c70f7
test: add sslcrl and sslcrldir to sensitive key test coverage
asheshv Apr 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
149 changes: 149 additions & 0 deletions web/migrations/versions/add_user_id_to_debugger_func_args_.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,149 @@
##########################################################################
#
# pgAdmin 4 - PostgreSQL Tools
#
# Copyright (C) 2013 - 2026, The pgAdmin Development Team
# This software is released under the PostgreSQL Licence
#
##########################################################################

"""Add user_id to debugger_function_arguments and indexes for data isolation

Revision ID: add_user_id_dbg_args
Revises: add_tools_ai_perm
Create Date: 2026-04-08

"""
from alembic import op
import sqlalchemy as sa

# revision identifiers, used by Alembic.
revision = 'add_user_id_dbg_args'
down_revision = 'add_tools_ai_perm'
branch_labels = None
depends_on = None


def upgrade():
conn = op.get_bind()
dialect = conn.dialect.name

# --- DebuggerFunctionArguments: add user_id to composite PK ---
if dialect == 'sqlite':
# SQLite cannot ALTER composite PKs. Recreate the table.
# Existing debugger argument data is ephemeral (cached function
# args) so dropping is acceptable.
op.execute(
'DROP TABLE IF EXISTS debugger_function_arguments'
)
op.create_table(
'debugger_function_arguments',
sa.Column('user_id', sa.Integer(),
sa.ForeignKey('user.id'), nullable=False),
sa.Column('server_id', sa.Integer(), nullable=False),
sa.Column('database_id', sa.Integer(), nullable=False),
sa.Column('schema_id', sa.Integer(), nullable=False),
sa.Column('function_id', sa.Integer(), nullable=False),
sa.Column('arg_id', sa.Integer(), nullable=False),
sa.Column('is_null', sa.Integer(), nullable=False),
sa.Column('is_expression', sa.Integer(), nullable=False),
sa.Column('use_default', sa.Integer(), nullable=False),
sa.Column('value', sa.String(), nullable=True),
sa.PrimaryKeyConstraint(
'user_id', 'server_id', 'database_id',
'schema_id', 'function_id', 'arg_id'
),
sa.CheckConstraint('is_null >= 0 AND is_null <= 1'),
sa.CheckConstraint(
'is_expression >= 0 AND is_expression <= 1'),
sa.CheckConstraint(
'use_default >= 0 AND use_default <= 1'),
)
else:
# PostgreSQL: add column, backfill from server owner, recreate
# PK using batch_alter_table for portability.
op.add_column(
'debugger_function_arguments',
sa.Column('user_id', sa.Integer(),
sa.ForeignKey('user.id'), nullable=True)
)
# Backfill: assign user_id from the server's owner
op.execute(
'UPDATE debugger_function_arguments '
'SET user_id = s.user_id '
'FROM server s '
'WHERE debugger_function_arguments.server_id = s.id'
)
# Delete orphans (rows with no matching server)
op.execute(
'DELETE FROM debugger_function_arguments '
'WHERE user_id IS NULL'
)
op.alter_column(
'debugger_function_arguments', 'user_id', nullable=False
)
# Recreate PK with user_id using batch_alter_table
with op.batch_alter_table(
'debugger_function_arguments'
) as batch:
batch.drop_constraint(
'debugger_function_arguments_pkey', type_='primary'
)
batch.create_primary_key(
'debugger_function_arguments_pkey',
['user_id', 'server_id', 'database_id',
'schema_id', 'function_id', 'arg_id']
)

# --- Indexes for data isolation query performance ---
# Only create indexes on tables that exist (sharedserver may be
# absent in older schemas that haven't run all prior migrations).
inspector = sa.inspect(conn)
index_stmts = [
('server',
'CREATE INDEX IF NOT EXISTS ix_server_user_id '
'ON server (user_id)'),
('server',
'CREATE INDEX IF NOT EXISTS ix_server_servergroup_id '
'ON server (servergroup_id)'),
('sharedserver',
'CREATE INDEX IF NOT EXISTS ix_sharedserver_user_id '
'ON sharedserver (user_id)'),
('sharedserver',
'CREATE INDEX IF NOT EXISTS ix_sharedserver_osid '
'ON sharedserver (osid)'),
('servergroup',
'CREATE INDEX IF NOT EXISTS ix_servergroup_user_id '
'ON servergroup (user_id)'),
]
for table_name, stmt in index_stmts:
if inspector.has_table(table_name):
op.execute(stmt)

# --- Unique constraint on SharedServer(osid, user_id) ---
# Prevents duplicate SharedServer records from TOCTOU race.
# First remove duplicates (keep lowest id per osid+user_id).
if inspector.has_table('sharedserver'):
if dialect == 'sqlite':
op.execute(
'DELETE FROM sharedserver WHERE id NOT IN '
'(SELECT MIN(id) FROM sharedserver '
'GROUP BY osid, user_id)'
)
else:
op.execute(
'DELETE FROM sharedserver s1 USING '
'sharedserver s2 WHERE s1.osid = s2.osid '
'AND s1.user_id = s2.user_id '
'AND s1.id > s2.id'
)
with op.batch_alter_table('sharedserver') as batch:
batch.create_unique_constraint(
'uq_sharedserver_osid_user',
['osid', 'user_id']
)


def downgrade():
# pgAdmin only upgrades, downgrade not implemented.
pass
11 changes: 4 additions & 7 deletions web/migrations/versions/ca00ec32581b_.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@

"""
from alembic import op
from sqlalchemy.orm.session import Session
from pgadmin.model import DebuggerFunctionArguments

# revision identifiers, used by Alembic.
revision = 'ca00ec32581b'
Expand All @@ -26,11 +24,10 @@


def upgrade():
session = Session(bind=op.get_bind())

debugger_records = session.query(DebuggerFunctionArguments).all()
if debugger_records:
session.delete(debugger_records)
# Use raw SQL instead of importing the model class, because
# model changes in later migrations (e.g. adding user_id) would
# cause this migration to fail on fresh databases.
op.execute('DELETE FROM debugger_function_arguments')


def downgrade():
Expand Down
35 changes: 20 additions & 15 deletions web/pgadmin/browser/server_groups/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@
from pgadmin.model import db, ServerGroup, Server
import config
from pgadmin.utils.preferences import Preferences
from pgadmin.utils.server_access import get_server_group, \
get_server_groups_for_user


def get_icon_css_class(group_id, group_user_id,
Expand Down Expand Up @@ -286,7 +288,7 @@ def update(self, gid):
def properties(self, gid):
"""Update the server-group properties"""

sg = ServerGroup.query.filter(ServerGroup.id == gid).first()
sg = get_server_group(gid)

if sg is None:
return make_json_response(
Expand All @@ -296,7 +298,8 @@ def properties(self, gid):
)
else:
return ajax_response(
response={'id': sg.id, 'name': sg.name, 'user_id': sg.user_id},
response={'id': sg.id, 'name': sg.name,
'user_id': sg.user_id},
status=200
)

Expand Down Expand Up @@ -373,8 +376,9 @@ def dependents(self, gid):
@staticmethod
def get_all_server_groups():
"""
Returns the list of server groups to show in server mode and
if there is any shared server in the group.
Returns the list of server groups to show in server mode.
Includes groups owned by the user and groups containing
shared servers accessible to this user.
:return: server groups
"""

Expand All @@ -383,17 +387,18 @@ def get_all_server_groups():
pref = Preferences.module('browser')
hide_shared_server = pref.preference('hide_shared_server').get()

server_groups = ServerGroup.query.all()
groups = []
for group in server_groups:
if hide_shared_server and \
ServerGroupModule.has_shared_server(group.id) and \
group.user_id != current_user.id:
continue
if group.user_id == current_user.id or \
ServerGroupModule.has_shared_server(group.id):
server_groups = get_server_groups_for_user()

if hide_shared_server:
groups = []
for group in server_groups:
if group.user_id != current_user.id and \
ServerGroupModule.has_shared_server(group.id):
continue
groups.append(group)
return groups
return groups

return server_groups

@pga_login_required
def nodes(self, gid=None):
Expand Down Expand Up @@ -421,7 +426,7 @@ def nodes(self, gid=None):
)
)
else:
group = ServerGroup.query.filter(ServerGroup.id == gid).first()
group = get_server_group(gid)

if not group:
return gone(
Expand Down
Loading
Loading