Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
27 changes: 26 additions & 1 deletion docs/domain/presentation-layer/specs/DESIGN.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand Down Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion docs/domain/presentation-layer/specs/PRD.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
11 changes: 11 additions & 0 deletions src/ingestion/scripts/apply-ch-migrations.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down
3 changes: 3 additions & 0 deletions src/ingestion/scripts/bootstrap-db/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
16 changes: 16 additions & 0 deletions src/ingestion/scripts/bootstrap-db/presentation-role.sql
Original file line number Diff line number Diff line change
@@ -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;
141 changes: 141 additions & 0 deletions src/ingestion/scripts/tests/test_presentation_role.py
Original file line number Diff line number Diff line change
@@ -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]
Comment thread
coderabbitai[bot] marked this conversation as resolved.
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
Loading