[Bug]: cohort query schema allowlist is bypassable by dropping the schema qualifier

[atriaybagur]

Description

validate_query enforces the omop schema allowlist only for schema-qualified table
references. Unqualified references skip the check entirely, so the allowlist is bypassed by
simply omitting the schema.

trust/data-access-api/data_access_api/services/cohort.py:162-167:

for table in stmt.find_all(exp.Table):
    schema_node = table.args.get("db")
    if schema_node is None:
        # Unqualified — Postgres resolves via search_path, which is set to
        # the omop schema in the OMOP DB image, so unqualified references
        # can only resolve to omop tables.
        continue

Verified against develop (69dd7f77):

REJECTED : SELECT * FROM pg_catalog.pg_class
ALLOWED  : SELECT * FROM pg_class
ALLOWED  : SELECT rolname FROM pg_roles
ALLOWED  : SELECT tablename FROM pg_tables
REJECTED : SELECT * FROM public.anything
ALLOWED  : SELECT * FROM omop.person

Reproduce:

cd trust/data-access-api
export DATA_ACCESS_POSTGRES_USER=u DATA_ACCESS_POSTGRES_PASSWORD=p OMOP_POSTGRES_DB=d
export AES_KEY_BASE64=$(python3 -c "import base64,os;print(base64.b64encode(os.urandom(32)).decode())")
export PYTHONPATH="$(pwd)/../observability:$(pwd)/data_access_api"
uv run python -c "
from data_access_api.services.cohort import validate_query
validate_query('SELECT * FROM pg_class')      # ALLOWED
validate_query('SELECT * FROM pg_catalog.pg_class')  # rejected
"

Why the stated justification does not hold

1. The search_path is never actually set. trust/omop-db/files/init.sql:13 issues a
   session-level SET search_path TO omop, public inside one initdb psql invocation. Nothing
   persists it — there is no ALTER DATABASE … SET search_path and no role-level setting. A live
   trust omop-db reports "$user", public, and SELECT count(*) FROM image_occurrence
   (unqualified) fails with "relation does not exist".
2. pg_catalog is implicitly in every search_path regardless of configuration, so
   pg_class / pg_roles / pg_tables resolve even if the intended search_path were applied.

Impact

A researcher on an approved project can enumerate the trust database catalogue — table names,
role names, schema layout — and reach unqualified names in public. data_analyst_reader is
read-only, so this is information disclosure rather than modification, and it requires an approved
project (insider path). It nonetheless defeats a control the code claims to enforce, and the
suppression threshold does not help: these queries return well over COHORT_QUERY_THRESHOLD rows.

Found while reviewing #837; the code is from #839 and is live on stag/prod.

Acceptance criteria

• Unqualified table references are either rejected outright, or resolved and checked against the allowlist
• The justification comment is corrected or removed (it is currently false in two independent ways)
• Test coverage for the unqualified form — tests/services/test_cohort.py currently covers only the qualified form
• Decide whether pg_catalog access should be blocked explicitly rather than relying on schema matching

Technical considerations

The trust is the authoritative layer here (the hub check is fast feedback only), so the fix belongs
in validate_query. Rejecting unqualified references outright is the simplest sound option, but it
would break any shipped tutorial or user query that omits the schema — worth grepping
fl-tutorials/ and the e2e query files before choosing.

[atriaybagur]

Correction: withdraw the search_path half of the argument

This issue argues the schema-pinning comment is wrong for two reasons. The second one does not
hold, and it should be withdrawn before someone uses it to dismiss the first.

The search_path is never actually set. trust/omop-db/files/init.sql:13 issues a
session-level SET search_path TO omop…

That reasons from #837's branch. On the Kubernetes provisioning path — the one in develop —
deploy/providers/kubernetes/templates/omop-db.yaml:174 does:

ALTER DATABASE "<db>" SET search_path = omop, public;

That is database-level, so it is in force for every connection.

The finding is unaffected. It stands entirely on the first argument, which is sound and
sufficient: Postgres searches pg_catalog implicitly and first, whatever search_path says. So
SELECT * FROM pg_class reaches the catalog however the database is configured. The verified
allowed/rejected matrix in the description is correct.

Fixed by #895 — by pinning rather than rejecting

#895 rewrites unqualified references to omop.<table> in the AST instead of refusing them.
validate_query already re-emits the query from the tree it validated, so a pinned node is a pinned
query, and omop.pg_class does not exist:

ALLOWED  : SELECT * FROM pg_class   ->  SELECT * FROM omop.pg_class
ALLOWED  : SELECT * FROM person     ->  SELECT * FROM omop.person

Rejecting would have been the smaller diff and the worse change — unqualified references are
ordinary in the cohort queries researchers have already saved, and (per the correction above)
search_path makes them work today, so rejecting would break working queries to close a hole that
pinning closes completely.

One thing this made visible

While fixing it: validate_query's docstring claimed the data_analyst_reader role holds "only
CONNECT + USAGE on schema omop". That is false on Kubernetes, where the chart grants
pg_read_all_data — SELECT on every table in every schema. So this check was not defence in depth
over a narrow grant; it was the only barrier, which makes this bug meaningfully worse than LOW on
that deployment path. Filed as #904; #895 corrects the documentation.