diff --git a/docs/domain/presentation-layer/specs/DESIGN.md b/docs/domain/presentation-layer/specs/DESIGN.md index 3ff024c48..37c3db74e 100644 --- a/docs/domain/presentation-layer/specs/DESIGN.md +++ b/docs/domain/presentation-layer/specs/DESIGN.md @@ -229,6 +229,31 @@ Makes the public query path incapable of anything but a single read, so a broken --- +#### Read-Only Role + +- [ ] `p2` - **ID**: `cpt-presentation-component-read-only-role` + +##### Why this component exists + +The second, independent barrier behind the query gate: once analytics connects as the role, even a read that slips past the gate executes under grants that make writing, altering, or dropping the source impossible. Read-only enforced by construction, not convention. The role is **provisioned** by #1963; it is **not yet the active query-path identity** — analytics still connects as the admin until that wiring lands, so this barrier is dormant until then. + +##### Responsibility scope + +- `presentation_ro` ClickHouse role: `SELECT` on the contract (silver, identity/person, legacy gold in `insight`); `SELECT`/`INSERT`/`CREATE` only in `presentation`; no `DROP`/`ALTER`/`TRUNCATE` anywhere. +- Defined as idempotent DDL in [presentation-role.sql](../../../../src/ingestion/scripts/bootstrap-db/presentation-role.sql); provisioned by [apply-ch-migrations.sh](../../../../src/ingestion/scripts/apply-ch-migrations.sh) (the clickhouse-migrate hook, which bootstrap also runs), guarded so a ClickHouse admin without access-management is skipped with a warning rather than aborting. + +##### Responsibility boundaries + +- Does NOT parse SQL — that is the query gate. +- Does NOT create the `presentation` database or wire the analytics connection to the role — those follow in #1964 and the connection wiring; until the connection wiring lands the role is provisioned but inactive. + +##### Related components (by ID) + +- `cpt-presentation-component-query-gate` — the first barrier; the role is the second +- `cpt-presentation-component-saved-query-api` — runs as this role + +--- + #### Saved-Query API - [ ] `p2` - **ID**: `cpt-presentation-component-saved-query-api` @@ -351,7 +376,7 @@ Entity `presentation.queries`: `{ id, insight_tenant_id, name, description, sql, | Presentation namespace | New `presentation` DB: `SELECT` + `CREATE`/`INSERT` for new gold, results, scratch | | Access | Executed as the `presentation_ro` role (SELECT on contract; CREATE/INSERT only in `presentation`) | | Read semantics | `FINAL` on silver `ReplacingMergeTree` reads | -| Bootstrap | Role and empty DB created in `src/ingestion/scripts/bootstrap-db/` | +| Bootstrap | Role defined in `src/ingestion/scripts/bootstrap-db/presentation-role.sql`, provisioned (guarded) by `apply-ch-migrations.sh`; the empty `presentation` DB follows in #1964 | #### Redis diff --git a/docs/domain/presentation-layer/specs/PRD.md b/docs/domain/presentation-layer/specs/PRD.md index e2d78b0e5..6c169ed46 100644 --- a/docs/domain/presentation-layer/specs/PRD.md +++ b/docs/domain/presentation-layer/specs/PRD.md @@ -165,7 +165,7 @@ The system **MUST** accept exactly one read statement — a single `SELECT`/`WIT - [ ] `p1` - **ID**: `cpt-presentation-fr-read-only-role` -The system **MUST** execute contract reads under a dedicated `presentation_ro` role that has `SELECT` on the silver and identity databases and `CREATE`/`INSERT` only in `presentation`, with no `DROP`/`ALTER`/`TRUNCATE` anywhere. (#1963.) +The system **MUST** execute contract reads under a dedicated `presentation_ro` role that has `SELECT` on the silver, identity, `person`, and legacy-gold (`insight`) databases and `CREATE`/`INSERT` only in `presentation`, with no `DROP`/`ALTER`/`TRUNCATE` anywhere. (#1963 provisions the role; it becomes the query-path identity once the analytics connection is wired to execute as it.) **Rationale**: Read-only enforced by construction, not by convention. diff --git a/src/ingestion/scripts/apply-ch-migrations.sh b/src/ingestion/scripts/apply-ch-migrations.sh index b10ee9568..332e86424 100755 --- a/src/ingestion/scripts/apply-ch-migrations.sh +++ b/src/ingestion/scripts/apply-ch-migrations.sh @@ -41,6 +41,17 @@ CREATE DATABASE IF NOT EXISTS silver; CREATE DATABASE IF NOT EXISTS ${CLICKHOUSE_DATABASE}; SQL +echo "=== Provisioning presentation_ro role (#1963) ===" +# Read-only role for the presentation query path (bootstrap-db/presentation-role.sql). +# Guarded + non-fatal: creating a role needs access_management on the admin, so an +# admin without it is skipped with a warning rather than aborting the deploy. +if printf 'CREATE ROLE IF NOT EXISTS presentation_ro' | _ch_http_query >/dev/null 2>&1; then + run_ch < "$SCRIPT_DIR/bootstrap-db/presentation-role.sql" + echo " presentation_ro ready" +else + echo " WARN: admin lacks access_management; skipping presentation_ro (see bootstrap-db/README.md)" +fi + echo "=== Creating bronze/silver placeholders (ADR-0007) ===" bash "$SCRIPT_DIR/create-bronze-placeholders.sh" diff --git a/src/ingestion/scripts/bootstrap-db/README.md b/src/ingestion/scripts/bootstrap-db/README.md index 2321ffa9a..a8790c761 100644 --- a/src/ingestion/scripts/bootstrap-db/README.md +++ b/src/ingestion/scripts/bootstrap-db/README.md @@ -19,9 +19,12 @@ Start a throwaway ClickHouse in docker, on the same version production runs (pin source pins.env docker run -d --name bootstrap-db-clickhouse -p 8123:8123 \ -e CLICKHOUSE_USER=insight -e CLICKHOUSE_PASSWORD=insight -e CLICKHOUSE_DB=insight \ + -e CLICKHOUSE_DEFAULT_ACCESS_MANAGEMENT=1 \ "${CLICKHOUSE_SERVER_IMAGE}" ``` +`CLICKHOUSE_DEFAULT_ACCESS_MANAGEMENT=1` lets the `insight` admin manage access (`CREATE ROLE`/`GRANT`) so the run provisions the read-only `presentation_ro` role (`presentation-role.sql`, #1963); the official image disables it by default. Both compose stacks (`docker-compose.yml`, `tests/e2e/compose`) and the bitnami prod admin already have access-management, and provisioning is guarded (an admin lacking it is skipped with a warning), so this flag is only needed for this bare throwaway container. + Point `.env` at it: `CLICKHOUSE_HOST=$(ipconfig getifaddr en0)` (the LAN IP — reachable both for dbt on this machine and for the connector containers; see Prerequisites), `CLICKHOUSE_PORT=8123`, `CLICKHOUSE_PROTOCOL=http`, user/password/database `insight`. Check what got created: ```bash diff --git a/src/ingestion/scripts/bootstrap-db/presentation-role.sql b/src/ingestion/scripts/bootstrap-db/presentation-role.sql new file mode 100644 index 000000000..b2a394479 --- /dev/null +++ b/src/ingestion/scripts/bootstrap-db/presentation-role.sql @@ -0,0 +1,16 @@ +-- presentation_ro: read-only-by-construction role for the presentation query +-- path (#1963). Contract = SELECT only; `presentation` = SELECT/INSERT/CREATE; +-- no DROP/ALTER/TRUNCATE anywhere. Idempotent. Needs an admin with +-- access_management (compose/e2e/bitnami already have it; see README.md). +-- Spec: docs/domain/presentation-layer/specs. + +CREATE ROLE IF NOT EXISTS presentation_ro; + +-- Contract (read-only): silver + identity/person + legacy gold in `insight`. +GRANT SELECT ON silver.* TO presentation_ro; +GRANT SELECT ON person.* TO presentation_ro; +GRANT SELECT ON identity.* TO presentation_ro; +GRANT SELECT ON insight.* TO presentation_ro; + +-- presentation (writable): no destructive DDL. +GRANT SELECT, INSERT, CREATE ON presentation.* TO presentation_ro; diff --git a/src/ingestion/scripts/tests/test_presentation_role.py b/src/ingestion/scripts/tests/test_presentation_role.py new file mode 100644 index 000000000..4d925fca1 --- /dev/null +++ b/src/ingestion/scripts/tests/test_presentation_role.py @@ -0,0 +1,141 @@ +"""Opt-in integration test: the `presentation_ro` role grant matrix (#1963). + +Pins the read-only-by-construction guarantee against a real ClickHouse: apply +bootstrap-db/presentation-role.sql, assign the role to a throwaway probe user, +and assert what that user can and cannot do. This is the adversarial-write half +of NFR `cpt-presentation-nfr-source-immutability` — the contract is read-only, +`presentation` is create/insert-only, and nothing can DROP/ALTER/TRUNCATE. + +Skipped unless a server is offered, so CI and local `pytest` stay dependency- +free. The admin must have access_management (to CREATE ROLE / CREATE USER): + + docker run -d --rm --name ch -p 38210:8123 \\ + -e CLICKHOUSE_USER=insight -e CLICKHOUSE_PASSWORD=insight \\ + -e CLICKHOUSE_DEFAULT_ACCESS_MANAGEMENT=1 \\ + clickhouse/clickhouse-server:25.7.5 + PRESENTATION_ROLE_TEST_CH_URL=http://localhost:38210 \\ + PRESENTATION_ROLE_TEST_CH_USER=insight \\ + PRESENTATION_ROLE_TEST_CH_PASSWORD=insight \\ + .venv/bin/python -m pytest tests/test_presentation_role.py -q +""" + +from __future__ import annotations + +import os +import urllib.error +import urllib.parse +import urllib.request +from pathlib import Path + +import pytest + +CH_URL = os.environ.get("PRESENTATION_ROLE_TEST_CH_URL") +CH_USER = os.environ.get("PRESENTATION_ROLE_TEST_CH_USER", "default") +CH_PASSWORD = os.environ.get("PRESENTATION_ROLE_TEST_CH_PASSWORD", "") + +pytestmark = pytest.mark.skipif( + not CH_URL, reason="set PRESENTATION_ROLE_TEST_CH_URL (admin with access_management) to run" +) + +ROLE_SQL = Path(__file__).resolve().parent.parent / "bootstrap-db" / "presentation-role.sql" + +# The read-only contract databases the role grants SELECT on. +CONTRACT_DBS = ("silver", "person", "identity", "insight") + +PROBE_USER = "pres_role_probe" +PROBE_PASSWORD = "probe" + + +def _query(sql: str, *, user: str, password: str) -> tuple[bool, str]: + """POST one statement over the HTTP interface. Returns (ok, body).""" + url = CH_URL.rstrip("/") + "/" + # Pin the scheme: urllib honours file:// etc. CH_URL is an operator-supplied + # test endpoint, but reject anything but http(s) so a stray value can't read + # local files. + if urllib.parse.urlparse(url).scheme not in ("http", "https"): + raise ValueError(f"PRESENTATION_ROLE_TEST_CH_URL must be http(s), got {CH_URL!r}") + req = urllib.request.Request( + url, data=sql.encode(), headers={"X-ClickHouse-User": user, "X-ClickHouse-Key": password} + ) + try: + # nosemgrep: python.lang.security.audit.dynamic-urllib-use-detected.dynamic-urllib-use-detected + with urllib.request.urlopen(req) as resp: # noqa: S310 (scheme pinned to http(s) above) + return True, resp.read().decode() + except urllib.error.HTTPError as e: + return False, e.read().decode() + + +def _admin(sql: str) -> tuple[bool, str]: + return _query(sql, user=CH_USER, password=CH_PASSWORD) + + +def _probe(sql: str) -> tuple[bool, str]: + return _query(sql, user=PROBE_USER, password=PROBE_PASSWORD) + + +def _apply(path: Path) -> None: + """Fan the SQL file out statement-by-statement, mirroring lib/ch-exec.sh + run_ch: drop full-line `--` comments, split on `;`.""" + body = "\n".join(line for line in path.read_text().splitlines() if not line.lstrip().startswith("--")) + for stmt in body.split(";"): + if stmt.strip(): + ok, resp = _admin(stmt) + assert ok, f"admin stmt failed: {stmt.strip()!r} -> {resp}" + + +@pytest.fixture(scope="module") +def probe(): + """Provision the contract + presentation objects and a probe user carrying + only the presentation_ro role. Torn down afterwards.""" + for db in (*CONTRACT_DBS, "presentation"): + assert _admin(f"CREATE DATABASE IF NOT EXISTS {db}")[0] + for db in CONTRACT_DBS: + assert _admin(f"CREATE TABLE IF NOT EXISTS {db}.probe (x UInt8) ENGINE=MergeTree ORDER BY x")[0] + + _apply(ROLE_SQL) + + # ClickHouse only accepts a default role once it is granted, so grant first. + assert _admin(f"DROP USER IF EXISTS {PROBE_USER}")[0] + ok, resp = _admin(f"CREATE USER {PROBE_USER} IDENTIFIED BY '{PROBE_PASSWORD}'") + assert ok, resp + assert _admin(f"GRANT presentation_ro TO {PROBE_USER}")[0] + assert _admin(f"ALTER USER {PROBE_USER} DEFAULT ROLE presentation_ro")[0] + try: + yield _probe + finally: + _admin("DROP TABLE IF EXISTS presentation.scratch") + for db in CONTRACT_DBS: + _admin(f"DROP TABLE IF EXISTS {db}.probe") + _admin(f"DROP USER IF EXISTS {PROBE_USER}") + + +@pytest.mark.parametrize("db", CONTRACT_DBS) +def test_contract_is_read_only(probe, db: str) -> None: + """SELECT on every contract database is allowed; every write/DDL is denied.""" + assert probe(f"SELECT count() FROM {db}.probe")[0], f"{db} SELECT must be allowed" + for sql in ( + f"INSERT INTO {db}.probe VALUES (1)", + f"DROP TABLE {db}.probe", + f"ALTER TABLE {db}.probe ADD COLUMN y UInt8", + f"TRUNCATE TABLE {db}.probe", + ): + ok, resp = probe(sql) + assert not ok, f"{db} must reject: {sql!r}" + assert "ACCESS_DENIED" in resp, resp + + +def test_presentation_is_create_insert_only(probe) -> None: + """CREATE/INSERT/SELECT allowed in presentation; DROP/ALTER/TRUNCATE denied.""" + assert probe("CREATE TABLE IF NOT EXISTS presentation.scratch (x UInt8) ENGINE=MergeTree ORDER BY x")[0], ( + "presentation CREATE must be allowed" + ) + assert probe("INSERT INTO presentation.scratch VALUES (7)")[0], "presentation INSERT must be allowed" + assert probe("SELECT sum(x) FROM presentation.scratch")[0], "presentation SELECT must be allowed" + for sql in ( + "DROP TABLE presentation.scratch", + "TRUNCATE TABLE presentation.scratch", + "ALTER TABLE presentation.scratch ADD COLUMN y UInt8", + ): + ok, resp = probe(sql) + assert not ok, f"presentation must reject destructive DDL: {sql!r}" + assert "ACCESS_DENIED" in resp, resp