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
2 changes: 1 addition & 1 deletion apps/api/src/cora/infrastructure/schema_version.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ class SchemaCheck:
expected: str


EXPECTED_SCHEMA_VERSION: Final = "20260810000000"
EXPECTED_SCHEMA_VERSION: Final = "20260810120000"
"""The newest migration this build was written against.

Hand-maintained, and deliberately not derived at runtime: the image does
Expand Down
115 changes: 75 additions & 40 deletions apps/api/tests/architecture/test_entries_table_grants.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,12 @@
PRIVILEGES" (see e.g. `20260621040000_init_entries_run_feed_heartbeats.sql`);
that claim is FALSE for tables (the role-init migration's
`ALTER DEFAULT PRIVILEGES` covers sequences only, per
`20260512230000_init_role_cora_app.sql`), so several tables created
this way carry no working grant at all. `_GRANDFATHERED` lists the
tables already affected; fixing them is a separate, already-tracked
follow-up (a GRANT-only migration against a live production database),
not something to silently paper over here. This test's job is to make
sure the mistake stops recurring: every table NOT on that list must
carry an explicit GRANT.
`20260512230000_init_role_cora_app.sql`). Five tables created this way
carried no working grant at all; `20260810120000_grant_cora_app_entries_table_access.sql`
closed the gap with a purely additive GRANT-only migration, so
`_GRANDFATHERED` is empty again. This test's job is to make sure the
mistake stops recurring: every entries_*/events table must carry an
explicit GRANT.
"""

from __future__ import annotations
Expand All @@ -29,49 +28,82 @@
r"CREATE\s+TABLE\s+(?:IF\s+NOT\s+EXISTS\s+)?([a-zA-Z_][a-zA-Z0-9_]*)",
re.IGNORECASE,
)

# Tables confirmed (2026-08-10, alongside the enclosure permit probe
# trail's own migration review) to rely on the false ALTER DEFAULT
# PRIVILEGES claim and carry no working GRANT today. Do not add to this
# list going forward: a new table belongs in a migration with its own
# explicit GRANT, per the assertion message below.
_GRANDFATHERED = frozenset(
{
"entries_run_readings",
"entries_operation_procedure_steps",
"entries_run_feed_heartbeats",
"entries_operation_procedure_diagnostics",
"entries_operation_procedure_outcomes",
}
_RENAME_TABLE_RE = re.compile(
r"ALTER\s+TABLE\s+([a-zA-Z_][a-zA-Z0-9_]*)\s+RENAME\s+TO\s+([a-zA-Z_][a-zA-Z0-9_]*)",
re.IGNORECASE,
)

# Closed: the five tables that relied on the false ALTER DEFAULT
# PRIVILEGES claim all got an explicit GRANT in
# 20260810120000_grant_cora_app_entries_table_access.sql. Do not add to
# this list going forward: a new table belongs in a migration with its
# own explicit GRANT, per the assertion message below.
_GRANDFATHERED: frozenset[str] = frozenset()


def _all_migration_text() -> str:
return "\n".join(f.read_text() for f in tracked_migration_files())


def _append_only_tables_created() -> set[str]:
out: set[str] = set()
def _append_only_tables_created() -> dict[str, frozenset[str]]:
"""Every CURRENTLY append-only table, keyed by its CURRENT identifier
and mapped to every name it has ever held.

Follows `ALTER TABLE ... RENAME TO ...` across ALL tables' migration
history, not just tables already matching `entries_`/`events`, then
filters to that prefix only on the final (current) name. A table can
enter the append-only family through a rename whose OLD name never
matched the prefix: `entries_conduit_verdicts` was created as
`observations_conduit_traversals`, renamed to
`entries_conduit_traversals`, then renamed again to its current name.
Gating the rename-follow on "old name already tracked as entries_/
events" would silently drop that chain the moment the origin name
fell outside the prefix, exactly the same blind spot this function
exists to close for the `entries_run_readings` case (see below), just
one hop earlier.

The same walk gives the full lineage, not just the current name,
which matters for the GRANT search: a privilege attaches to the
table's OID, not its name, so a GRANT issued under an OLD name (e.g.
`entries_decision_reasonings`, before it became
`entries_decision_inferences`) remains valid forever and a rename
never needs it re-issued under the new name. And the current-name
requirement matters for correctness in the other direction: a GRANT
written TODAY must target the table's current name (e.g.
`entries_run_observations`, not the dead `entries_run_readings`), the
only name that actually exists in the database by the time a later
migration runs.
"""
lineage: dict[str, set[str]] = {}
for path in tracked_migration_files():
for match in _CREATE_TABLE_RE.finditer(path.read_text()):
text = path.read_text()
for match in _CREATE_TABLE_RE.finditer(text):
name = match.group(1)
if name == "events" or name.startswith("entries_"):
out.add(name)
return out
lineage.setdefault(name, {name})
for match in _RENAME_TABLE_RE.finditer(text):
old_name, new_name = match.group(1), match.group(2)
if old_name in lineage:
names = lineage.pop(old_name)
names.add(new_name)
lineage[new_name] = names
return {
name: frozenset(names)
for name, names in lineage.items()
if name == "events" or name.startswith("entries_")
}


@pytest.mark.architecture
def test_every_new_entries_table_has_cora_app_grant() -> None:
"""Pattern accepted: `GRANT ... ON [TABLE] <table> ... TO cora_app`.

Tables on `_GRANDFATHERED` are skipped: they predate this test and
fixing them is a separate production migration, not a test change.
A table that is renamed off the grandfathered list (its CREATE TABLE
name changes) is NOT exempt under its new name; only the exact
grandfathered identifiers are excused.
"""Pattern accepted: `GRANT ... ON [TABLE] <name> ... TO cora_app`,
where `<name>` is any name in the table's rename lineage (see
`_append_only_tables_created`), not just its current one. Tables on
`_GRANDFATHERED` are skipped: they predate this test and fixing them
is a separate production migration, not a test change.
"""
haystack = _all_migration_text()
tables = _append_only_tables_created() - _GRANDFATHERED
lineages = _append_only_tables_created()
tables = set(lineages) - _GRANDFATHERED
assert tables, (
"No non-grandfathered append-only tables found; either the schema "
"is empty, table-name detection is wrong, or _GRANDFATHERED has "
Expand All @@ -80,11 +112,14 @@ def test_every_new_entries_table_has_cora_app_grant() -> None:

missing: list[str] = []
for table in sorted(tables):
pattern = re.compile(
rf"GRANT\b[^;]*\bON\s+(?:TABLE\s+)?{re.escape(table)}\b[^;]*\bTO\s+[^;]*cora_app\b",
re.IGNORECASE | re.DOTALL,
)
if not pattern.search(haystack):
if not any(
re.search(
rf"GRANT\b[^;]*\bON\s+(?:TABLE\s+)?{re.escape(name)}\b[^;]*\bTO\s+[^;]*cora_app\b",
haystack,
re.IGNORECASE | re.DOTALL,
)
for name in lineages[table]
):
missing.append(table)

assert not missing, (
Expand Down
88 changes: 87 additions & 1 deletion apps/api/tests/integration/test_cora_app_role_revoke_postgres.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
"""Integration test: cora_app role cannot UPDATE / DELETE append-only tables.
"""Integration test: cora_app role cannot UPDATE / DELETE append-only tables,
and DOES have SELECT + INSERT on every entries_* table.

Foundation hardening. The migration
`20260512230000_init_role_cora_app.sql` creates a `cora_app` database
Expand All @@ -15,6 +16,11 @@
review (Postgres now refuses them at the role boundary)
- The classic event-sourcing failure mode where "events are
immutable" is documented but not enforced
- A GRANT statement that exists in migration text but that Postgres
does not actually honor (`test_entries_table_grants.py` is a regex
match against SQL source, not a proof the database accepts it; five
tables carried a false ALTER DEFAULT PRIVILEGES claim for months
with a passing test suite before `20260810120000_...sql` fixed it)

The fixtures in `conftest.py` connect as the testcontainers
superuser so the rest of the suite can TRUNCATE between tests.
Expand Down Expand Up @@ -192,6 +198,86 @@ async def test_cora_app_cannot_update_or_delete_entries_tables(
await conn.execute(f"DELETE FROM {table}")


_ENTRIES_TABLE_INSERTS: dict[str, tuple[str, int]] = {
"entries_run_observations": (
"""
INSERT INTO entries_run_observations (
event_id, run_id, logbook_id, actor_id, command_name,
channel_name, value, sampling_procedure, sampled_at,
occurred_at, correlation_id
) VALUES ($1, $2, $3, $4, 'TestCmd', 'test-channel', 1.0,
'periodic', now(), now(), $5)
""",
5,
),
"entries_operation_procedure_activities": (
"""
INSERT INTO entries_operation_procedure_activities (
event_id, procedure_id, logbook_id, actor_id, command_name,
step_kind, payload, sampled_at, occurred_at, correlation_id
) VALUES ($1, $2, $3, $4, 'TestCmd', 'SetpointStep', '{}'::jsonb,
now(), now(), $5)
""",
5,
),
"entries_run_feed_heartbeats": (
"""
INSERT INTO entries_run_feed_heartbeats (
event_id, run_id, source_id, heartbeat_at
) VALUES ($1, $2, 'test-source', now())
""",
2,
),
"entries_operation_procedure_diagnostics": (
"""
INSERT INTO entries_operation_procedure_diagnostics (
event_id, procedure_id, logbook_id, iteration_index,
model_ref, payload, sampled_at, occurred_at, correlation_id
) VALUES ($1, $2, $3, 1, 'test-model', '{}'::jsonb, now(), now(), $4)
""",
4,
),
"entries_operation_procedure_outcomes": (
"""
INSERT INTO entries_operation_procedure_outcomes (
event_id, procedure_id, logbook_id, iteration_index,
point, measurements, succeeded, sampled_at, occurred_at,
correlation_id
) VALUES ($1, $2, $3, 1, '{}'::jsonb, '{}'::jsonb, true, now(), now(), $4)
""",
4,
),
}
"""The five tables `20260810120000_grant_cora_app_entries_table_access.sql`
fixed, by their CURRENT (post-rename) name, each mapped to a minimal
valid INSERT and its UUID-parameter count. Column shapes copied from
each table's own CREATE TABLE migration; `is_simulated` and other
DEFAULT-bearing columns are omitted since a bare INSERT already proves
the grant."""


@pytest.mark.integration
@pytest.mark.parametrize("table", sorted(_ENTRIES_TABLE_INSERTS))
async def test_cora_app_can_select_and_insert_entries_tables(
cora_app_pool: asyncpg.Pool,
table: str,
) -> None:
"""The regression this test exists for: each of these five tables'
migration header claimed cora_app already had SELECT + INSERT via
ALTER DEFAULT PRIVILEGES, a claim that was false (that clause covers
sequences only). `test_entries_table_grants.py` proves a GRANT
statement is present in migration text; only a real INSERT against a
`cora_app`-credentialed pool proves Postgres actually honors it."""
sql, param_count = _ENTRIES_TABLE_INSERTS[table]
event_id = uuid4()
params = [event_id, *(uuid4() for _ in range(param_count - 1))]

async with cora_app_pool.acquire() as conn:
await conn.execute(sql, *params)
rows = await conn.fetch(f"SELECT event_id FROM {table} WHERE event_id = $1", event_id)
assert len(rows) == 1


@pytest.mark.integration
async def test_cora_app_can_mutate_idempotency_keys(
cora_app_pool: asyncpg.Pool,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
-- Fixes a latent permission gap: five entries_* tables were created
-- with a header comment claiming cora_app "gets SELECT + INSERT via
-- ALTER DEFAULT PRIVILEGES in 20260512230000_init_role_cora_app.sql".
-- That claim is false. The role-init migration's
-- `ALTER DEFAULT PRIVILEGES ... GRANT USAGE, SELECT ON SEQUENCES`
-- covers SEQUENCES only, never TABLES, so cora_app has never had a
-- working grant on any of these tables. Confirmed by grepping every
-- migration for a GRANT naming each table plus cora_app: none exists.
--
-- Affected tables, named by their CURRENT identifier (two of the five
-- were renamed after creation, so the GRANT below must target the name
-- the table actually holds by this point in migration history, not the
-- name it was created under):
-- entries_run_readings -> entries_run_observations
-- (20260610020000_rename_entries_run_readings_to_entries_run_observations.sql)
-- entries_operation_procedure_steps -> entries_operation_procedure_activities
-- (20260610030000_rename_entries_operation_procedure_steps_to_entries_operation_procedure_activities.sql)
-- entries_run_feed_heartbeats (unrenamed)
-- entries_operation_procedure_diagnostics (unrenamed)
-- entries_operation_procedure_outcomes (unrenamed)
--
-- Each already carries its own REVOKE UPDATE, DELETE, TRUNCATE (a
-- privilege revocation attaches to the table object and survives a
-- later rename), so only the missing GRANT is added here.
--
-- Purely additive and non-destructive: a GRANT cannot lock a table or
-- fail against existing rows. Currently dormant, not live, because the
-- pilot's DATABASE_URL still connects as the owner role (cora), not
-- cora_app; this closes the gap before anything switches to the
-- restricted role. See tests/architecture/test_entries_table_grants.py,
-- whose _GRANDFATHERED allowlist these five names are removed from in
-- the same change as this migration.

GRANT SELECT, INSERT ON entries_run_observations TO cora_app;
GRANT SELECT, INSERT ON entries_operation_procedure_activities TO cora_app;
GRANT SELECT, INSERT ON entries_run_feed_heartbeats TO cora_app;
GRANT SELECT, INSERT ON entries_operation_procedure_diagnostics TO cora_app;
GRANT SELECT, INSERT ON entries_operation_procedure_outcomes TO cora_app;
3 changes: 2 additions & 1 deletion infra/atlas/migrations/atlas.sum
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
h1:bdALzYDPVBnS+sQ6HlFKoV4ZwWy02BiLdJGYVeBbgGg=
h1:Hh2aacDC3e9ka8TTvNmeD8Zw7LlHWQSMl8waDuzuWL0=
20260509120000_init_events.sql h1:GmgCZKfaqXu1m96/cKAks2vhaLWTdEaHTLkFtUo9FXg=
20260509170000_init_idempotency.sql h1:Nbu8DIE4Sv1WiHw3G22+tYffPhKc5Jryw3PMK8wB2zY=
20260510010000_add_event_id.sql h1:RbtYP6uMnOB20zhJ9dNXUi4YVqbmlEzf562pmygnRW8=
Expand Down Expand Up @@ -162,3 +162,4 @@ h1:bdALzYDPVBnS+sQ6HlFKoV4ZwWy02BiLdJGYVeBbgGg=
20260729120000_add_proj_data_dataset_summary_checksum.sql h1:Zk3BmIBMjck0Q4OkKzP1wU93NLWC2b5IRV0iM0ihlX0=
20260809190000_split_enclosure_permit_transition_and_source_times.sql h1:xjTv+1Eq6Ujg40E4y0mOK5nIlT8pXKhdp3ve7No+uvQ=
20260810000000_init_entries_enclosure_permit_probes.sql h1:jSvy8jXhHwZbq1A4AMqKEL3uviaJtlyGLCGvISfevd4=
20260810120000_grant_cora_app_entries_table_access.sql h1:LCngXYBMYN5aS7fy/3Sa8G1C/9KNG+J4GFf5Xh4X2kM=
Loading