diff --git a/frontend/package-lock.json b/frontend/package-lock.json index c23efbbc5..a24cc87ab 100644 --- a/frontend/package-lock.json +++ b/frontend/package-lock.json @@ -1405,6 +1405,12 @@ "url": "https://opencollective.com/libvips" } }, + "node_modules/@isaacs/nonrecursive-glob-match": { + "version": "1.0.3", + "resolved": "https://registry.npmjs.org/@isaacs/nonrecursive-glob-match/-/nonrecursive-glob-match-1.0.3.tgz", + "integrity": "sha512-SI7kyKHq4XXDRozw+r9GlRZkia1ZkFrpLW98r2IC2Bpr8YkFGE3fvjzmxCG0x78A++69HmDNrDvSMYBZmHGpKA==", + "license": "BlueOak-1.0.0" + }, "node_modules/@jest/diff-sequences": { "version": "30.0.1", "resolved": "https://registry.npmjs.org/@jest/diff-sequences/-/diff-sequences-30.0.1.tgz", @@ -7866,9 +7872,9 @@ "license": "ISC" }, "node_modules/follow-redirects": { - "version": "1.16.1", - "resolved": "https://registry.npmjs.org/follow-redirects/-/follow-redirects-1.16.1.tgz", - "integrity": "sha512-FNvFGzoMLWmE6Yj9spb/zjd7yiNCHiAW9/Tg9CXrQ8wuu32HtlJOwWO11OJafl5FfY3DxTdQ0vj42zU1kvv5jg==", + "version": "1.16.2", + "resolved": "https://registry.npmjs.org/follow-redirects/-/follow-redirects-1.16.2.tgz", + "integrity": "sha512-/kVP2U8z596Ia0gwFrwyIeo74ta5aVmSeU1FfAUMAmcEP5EP7zydmag3SLiN64NaRKef3lxsq7i/g7ltJQDGow==", "funding": [ { "type": "individual", @@ -9823,12 +9829,13 @@ } }, "node_modules/minimatch": { - "version": "10.2.4", - "resolved": "https://registry.npmjs.org/minimatch/-/minimatch-10.2.4.tgz", - "integrity": "sha512-oRjTw/97aTBN0RHbYCdtF1MQfvusSIBQM0IZEgzl6426+8jSC0nF1a/GmnVLpfB9yyr6g6FTqWqiZVbxrtaCIg==", + "version": "11.0.3", + "resolved": "https://registry.npmjs.org/minimatch/-/minimatch-11.0.3.tgz", + "integrity": "sha512-iny/40PEGkNJQ5/Pv6z3nm6FbRAby32GeZRjCUJk2WnNDbgPSTcdI+DDvLqsgBdD4cD5sMtMheqPPnehCCa8Nw==", "license": "BlueOak-1.0.0", "dependencies": { - "brace-expansion": "^5.0.2" + "@isaacs/nonrecursive-glob-match": "^1.0.3", + "brace-expansion": "^5.0.12" }, "engines": { "node": "18 || 20 || >=22" diff --git a/frontend/package.json b/frontend/package.json index 7fbbfb069..f754526bd 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -105,7 +105,8 @@ "rollup": "4.59.0", "esbuild": "0.27.3", "fast-uri": "^3.1.5", - "minimatch": ">=3.1.3", + "follow-redirects": ">=1.16.2", + "minimatch": ">=11.0.1", "postcss": "^8.5.18", "serialize-javascript": "7.0.3", "workbox-build": { diff --git a/packages/factory-local/requirements.lock b/packages/factory-local/requirements.lock index 925eeb74c..700b730d0 100644 --- a/packages/factory-local/requirements.lock +++ b/packages/factory-local/requirements.lock @@ -1235,9 +1235,9 @@ pygments==2.21.0 \ --hash=sha256:2363c69b61c4a97c838da3b130dcd6468f4848992b21a82f2a63ec34377137d9 \ --hash=sha256:610ca751c9bc2492b38eb9a38a7fbc93edbbb2d7182edaf34e66ae493dee5c8c # via pytest -pypdf==6.21.2 \ - --hash=sha256:0a3e1c5c886a1be03646c16a04827cc166c2a0575c971b26a4bf89686551c2d4 \ - --hash=sha256:1d44d1a3a200f25062adff0085b02040e4646431ac2ccac22e42679e91466084 +pypdf==6.21.4 \ + --hash=sha256:33e4fb41ca352810eb4e9838aa0d82d6888a5f8bf682f43e99cce5faa666255c \ + --hash=sha256:c8817e3f888d47a087872ded69812c1ef30cacb871d6e053e0c00bbaf0bc2163 # via -r requirements.txt pytest==9.1.1 \ --hash=sha256:1088fbde8f2b49d95a549a195707afa7a76a3ce9bcadc26b6d71f0ffda5fe313 \ diff --git a/packages/factory-local/requirements.txt b/packages/factory-local/requirements.txt index 890ce9304..778f4f886 100644 --- a/packages/factory-local/requirements.txt +++ b/packages/factory-local/requirements.txt @@ -8,6 +8,6 @@ neo4j>=5.23,<6 streamlit>=1.38,<2 pytest>=9.0.3,<10 beautifulsoup4>=4.12,<5 -pypdf>=6.21.2,<7 +pypdf>=6.21.4,<7 openai>=1.40,<3 python-dotenv>=1.0,<2 diff --git a/requirements.in b/requirements.in index 7d228d4e5..0d6a98310 100644 --- a/requirements.in +++ b/requirements.in @@ -163,7 +163,7 @@ beautifulsoup4>=4.15.0 lxml>=6.1.3 pdfminer.six>=20260107 pdfplumber>=0.11.10 -pypdf>=6.21.2,<7 # CVE-2026-115572/115580/115585; CVE-2026-71852, CVE-2026-71870; earlier: CVE-2026-59935/59936 (inline-image DoS), CVE-2026-48155/48156, GHSA-jm82-fx9c-mx94 +pypdf>=6.21.4,<7 # CVE-2026-119203/119204/119205 (6.21.4); CVE-2026-119206/119207/119208/119209 (6.21.3); earlier: CVE-2026-115572/115580/115585, CVE-2026-71852/71870, CVE-2026-59935/59936, CVE-2026-48155/48156, GHSA-jm82-fx9c-mx94 fpdf2>=2.8.9,<3 python-docx>=1.2.0 docx2txt>=0.9 diff --git a/requirements.lock b/requirements.lock index 98bb01f8b..133aa6865 100644 --- a/requirements.lock +++ b/requirements.lock @@ -2999,9 +2999,9 @@ psycopg-binary==3.3.6 \ --hash=sha256:fa34eb47969297471db7b7f193622c7e3ee839ec05abd05f1fe104d5b1b1dcf4 \ --hash=sha256:fdccb3a0e184b03e9baa673b15a809cf36c339c85dbda0ebc25a698846dfbee8 # via psycopg -pyasn1==0.6.4 \ - --hash=sha256:9c447d8431c947fe4c8febc4ed9e760bc29011a5b01e5c74b67025bd9fb8ce81 \ - --hash=sha256:deda9277cfd454080ec40b207fb6df82206a3a2688735233cdcd8d3d565f088b +pyasn1==0.6.5 \ + --hash=sha256:4017b0fce125096b8573b39c989fdb1a4b19cf761e683398184550fea0e66587 \ + --hash=sha256:6b11616a6bcf3a40e635f082bb232fa57b5e0dd7f517bd7ddd69ec3794508a47 # via # pyasn1-modules # webauthn @@ -3202,9 +3202,9 @@ pyotp==2.10.0 \ --hash=sha256:1df2f6a1bcc3bb0716172a5215ddc2f8c7c7fd26a13df9927d52e1746934836c \ --hash=sha256:d01e9703443616b03c57c700b5cbffd56a1f929c1b0f8f03131bc78c1fca9d3f # via -r requirements.in -pypdf==6.21.2 \ - --hash=sha256:0a3e1c5c886a1be03646c16a04827cc166c2a0575c971b26a4bf89686551c2d4 \ - --hash=sha256:1d44d1a3a200f25062adff0085b02040e4646431ac2ccac22e42679e91466084 +pypdf==6.21.4 \ + --hash=sha256:33e4fb41ca352810eb4e9838aa0d82d6888a5f8bf682f43e99cce5faa666255c \ + --hash=sha256:c8817e3f888d47a087872ded69812c1ef30cacb871d6e053e0c00bbaf0bc2163 # via -r requirements.in pypdfium2==5.13.0 \ --hash=sha256:07f58e91b8c45ca144a1ff3008faf3c73ef8a5e9fb32988831788363288228cd \ diff --git a/services/admin/app/user_routes.py b/services/admin/app/user_routes.py index 95b7dfc55..851c7578b 100644 --- a/services/admin/app/user_routes.py +++ b/services/admin/app/user_routes.py @@ -11,7 +11,7 @@ from sqlalchemy.orm import Session from .audit import AuditLogger -from .database import get_session +from .database import arm_global_admin_context, disarm_global_admin_context, get_session from .dependencies import PermissionChecker, get_current_user from .models import TenantContext from .sqlalchemy_models import MembershipModel, RoleModel, UserModel @@ -120,45 +120,48 @@ async def list_users( # drops every co-member and the directory returns only the caller. Permission # Checker("users.read") already authorized this request; keep the query # scoped by MembershipModel.tenant_id so the arm does not widen tenants. - # set_admin_context(true) is a no-op under SQLite. - TenantContext.set_admin_context(db, True) - - # Count total - count_stmt = ( - select(func.count()) - .select_from(UserModel) - .join(MembershipModel, UserModel.id == MembershipModel.user_id) - .where(MembershipModel.tenant_id == tenant_id) - ) - total = db.execute(count_stmt).scalar() - - # helper for explicit join - stmt = ( - select(UserModel, MembershipModel, RoleModel) - .join(MembershipModel, UserModel.id == MembershipModel.user_id) - .join(RoleModel, MembershipModel.role_id == RoleModel.id) - .where(MembershipModel.tenant_id == tenant_id) - .offset(pagination.skip) - .limit(pagination.limit) - ) - - rows = db.execute(stmt).all() + # Use transaction-local SET LOCAL (arm/disarm) so the GUC cannot leak past + # this handler if the pooled connection is reused before checkout reset. + arm_global_admin_context(db) + try: + # Count total + count_stmt = ( + select(func.count()) + .select_from(UserModel) + .join(MembershipModel, UserModel.id == MembershipModel.user_id) + .where(MembershipModel.tenant_id == tenant_id) + ) + total = db.execute(count_stmt).scalar() + + # helper for explicit join + stmt = ( + select(UserModel, MembershipModel, RoleModel) + .join(MembershipModel, UserModel.id == MembershipModel.user_id) + .join(RoleModel, MembershipModel.role_id == RoleModel.id) + .where(MembershipModel.tenant_id == tenant_id) + .offset(pagination.skip) + .limit(pagination.limit) + ) - results = [] - for user, membership, role in rows: - results.append( - UserResponse( - id=user.id, - email=user.email, - status="inactive" if not membership.is_active else user.status, - is_sysadmin=user.is_sysadmin, - role_id=role.id, - role_name=role.name, - created_at=user.created_at, + rows = db.execute(stmt).all() + + results = [] + for user, membership, role in rows: + results.append( + UserResponse( + id=user.id, + email=user.email, + status="inactive" if not membership.is_active else user.status, + is_sysadmin=user.is_sysadmin, + role_id=role.id, + role_name=role.name, + created_at=user.created_at, + ) ) - ) - return PaginatedResponse(items=results, total=total, skip=pagination.skip, limit=pagination.limit) + return PaginatedResponse(items=results, total=total, skip=pagination.skip, limit=pagination.limit) + finally: + disarm_global_admin_context(db) @router.patch( diff --git a/services/admin/tests/test_user_routes_1342.py b/services/admin/tests/test_user_routes_1342.py index 3f5979c8d..f9f955d56 100644 --- a/services/admin/tests/test_user_routes_1342.py +++ b/services/admin/tests/test_user_routes_1342.py @@ -103,14 +103,15 @@ def _execute_side_effect(*args, **kwargs): call_count[0] += 1 result = MagicMock() if call_count[0] == 1: - # set_admin_context arm (users FORCE RLS peer visibility) + # arm_global_admin_context (SET LOCAL is_sysadmin) return result if call_count[0] == 2: # Count query result.scalar.return_value = 1 - else: + elif call_count[0] == 3: # Rows query result.all.return_value = [(user, membership, role)] + # call 4: disarm_global_admin_context in finally return result db.execute.side_effect = _execute_side_effect @@ -147,7 +148,7 @@ def _execute_side_effect(*args, **kwargs): return result if call_count[0] == 2: result.scalar.return_value = 1 - else: + elif call_count[0] == 3: result.all.return_value = [(user, membership, role)] return result @@ -177,7 +178,7 @@ def _execute_side_effect(*args, **kwargs): return result if call_count[0] == 2: result.scalar.return_value = 0 - else: + elif call_count[0] == 3: result.all.return_value = [] return result diff --git a/tests/security/test_global_admin_context_allowlist.py b/tests/security/test_global_admin_context_allowlist.py index be48a73c4..8119d1606 100644 --- a/tests/security/test_global_admin_context_allowlist.py +++ b/tests/security/test_global_admin_context_allowlist.py @@ -92,6 +92,13 @@ "identity-global-self-service", 1, ), + # Tenant-scoped directory under PermissionChecker("users.read"). + # Arm unlocks users_self_or_admin peer rows; MembershipModel.tenant_id + # still filters. SET LOCAL via arm/disarm (try/finally). + ("services/admin/app/user_routes.py", "list_users"): ( + "tenant-scoped-directory", + 1, + ), # Background coordination paths. These run only on ADMIN_DATABASE_URL and # do not derive global authority from request metadata. ("services/shared/task_queue.py", "_arm_global_queue_transaction"): ( @@ -191,5 +198,6 @@ def test_allowlist_uses_only_reviewed_authority_classes(): "global-human", "global-human-or-workload", "identity-global-self-service", + "tenant-scoped-directory", "worker-maintenance", } diff --git a/tests/shared/test_list_users_tenant_isolation_live.py b/tests/shared/test_list_users_tenant_isolation_live.py new file mode 100644 index 000000000..f27a98d14 --- /dev/null +++ b/tests/shared/test_list_users_tenant_isolation_live.py @@ -0,0 +1,146 @@ +"""Live-Postgres: list_users returns only the caller's tenant members. + +After #3749, list_users arms regengine.is_sysadmin so public.users FORCE RLS +(users_self_or_admin) can see co-members. That arm must not widen the +directory across tenants: MembershipModel.tenant_id still filters the join, +and the admin GUC is transaction-local (SET LOCAL via arm/disarm). + +This test seeds two tenants with disjoint synthetic members, invokes +list_users under each tenant context as regengine_sysadmin, and asserts +each call returns only that tenant's emails. Requires a designated live +test DB (same guards as the other tests/shared/*_live.py suite). +""" + +from __future__ import annotations + +import asyncio +import uuid + +import pytest +from shared.db_url import sync_postgres_url +from shared.pagination import PaginationParams +from sqlalchemy import create_engine, text +from sqlalchemy.orm import Session + +from tests.migrations.rls_db import guarded_live_database_url + +DATABASE_URL = guarded_live_database_url() +pytestmark = pytest.mark.skipif( + not DATABASE_URL, + reason=( + "no designated live test DB (CI DATABASE_URL or REGENGINE_MIGRATION_TEST_DB) " + "— skipping list_users tenant isolation live test" + ), +) + + +def _engine(): + return create_engine(sync_postgres_url(DATABASE_URL)) + + +def _seed_tenant(conn, *, label: str) -> tuple[uuid.UUID, set[str]]: + """Insert one tenant with two active members. Caller rolls back.""" + tenant_id = uuid.uuid4() + role_id = uuid.uuid4() + suffix = uuid.uuid4().hex[:12] + emails: set[str] = set() + + conn.execute( + text("INSERT INTO public.tenants (id, name, slug) VALUES (:id, :name, :slug)"), + {"id": str(tenant_id), "name": f"{label}-{suffix}", "slug": f"{label}-{suffix}"}, + ) + conn.execute( + text("INSERT INTO public.roles (id, tenant_id, name) VALUES (:id, :tid, 'Member')"), + {"id": str(role_id), "tid": str(tenant_id)}, + ) + + for idx in (1, 2): + user_id = uuid.uuid4() + email = f"{label}-user{idx}-{suffix}@example.test" + emails.add(email) + conn.execute( + text( + "INSERT INTO public.users (id, email, status) VALUES (:id, :email, 'active')" + ), + {"id": str(user_id), "email": email}, + ) + conn.execute( + text( + """ + INSERT INTO public.memberships (user_id, tenant_id, role_id, is_active) + VALUES (:uid, :tid, :rid, true) + """ + ), + {"uid": str(user_id), "tid": str(tenant_id), "rid": str(role_id)}, + ) + + return tenant_id, emails + + +def test_list_users_returns_only_caller_tenant_members() -> None: + from services.admin.app import user_routes as ur + from services.admin.app.models import TenantContext + + engine = _engine() + try: + with engine.connect() as conn: + trans = conn.begin() + try: + role_exists = conn.execute( + text("SELECT 1 FROM pg_roles WHERE rolname = 'regengine_sysadmin'") + ).scalar() + if not role_exists: + pytest.skip("regengine_sysadmin role not present in this database") + + policy_exists = conn.execute( + text( + "SELECT 1 FROM pg_policies WHERE schemaname = 'public' " + "AND tablename = 'users' " + "AND policyname = 'users_self_or_admin'" + ) + ).scalar() + assert policy_exists, ( + "users_self_or_admin policy missing — list_users peer visibility " + "depends on that FORCE RLS arm" + ) + + tenant_a, emails_a = _seed_tenant(conn, label="lista") + tenant_b, emails_b = _seed_tenant(conn, label="listb") + assert emails_a.isdisjoint(emails_b) + + # Seed as the migration owner; evaluate RLS as the admin engine role. + conn.execute(text("SET LOCAL ROLE regengine_sysadmin")) + + db = Session(bind=conn) + + TenantContext.set_tenant_context(db, tenant_a) + result_a = asyncio.run( + ur.list_users( + pagination=PaginationParams(skip=0, limit=50), + db=db, + ) + ) + got_a = {item.email for item in result_a.items} + assert got_a == emails_a, ( + f"tenant A directory leaked or missed members: got={got_a} expected={emails_a}" + ) + assert result_a.total == len(emails_a) + assert got_a.isdisjoint(emails_b) + + TenantContext.set_tenant_context(db, tenant_b) + result_b = asyncio.run( + ur.list_users( + pagination=PaginationParams(skip=0, limit=50), + db=db, + ) + ) + got_b = {item.email for item in result_b.items} + assert got_b == emails_b, ( + f"tenant B directory leaked or missed members: got={got_b} expected={emails_b}" + ) + assert result_b.total == len(emails_b) + assert got_b.isdisjoint(emails_a) + finally: + trans.rollback() + finally: + engine.dispose()