From 409f806aae12b88cb49bfd7b250dda4fd8303a83 Mon Sep 17 00:00:00 2001 From: mptyl Date: Sun, 12 Jul 2026 01:13:56 +0200 Subject: [PATCH] fix(vector): verify writer sequence privileges --- .superpowers/sdd/pgvector-task-1-report.md | 20 ++++++++ harness/tests/l0/test_pgvector_store.py | 56 ++++++++++++++++++---- harness/tht/adapters/vector/pgvector.py | 30 +++++++++++- 3 files changed, 96 insertions(+), 10 deletions(-) diff --git a/.superpowers/sdd/pgvector-task-1-report.md b/.superpowers/sdd/pgvector-task-1-report.md index 96832d39..f2025c83 100644 --- a/.superpowers/sdd/pgvector-task-1-report.md +++ b/.superpowers/sdd/pgvector-task-1-report.md @@ -73,3 +73,23 @@ Fresh verification after the fix wave: - Expanded focused adapter/config suite: `56 passed`. - Full harness: `466 passed, 5 deselected`. - Changed-file Ruff lint/format and `git diff --check`: clean. + +## Sequence privilege health follow-up + +Writer health now resolves the real serial/identity sequence for the `id` column of every +required collection using `pg_get_serial_sequence`. It requires `USAGE` on each resolved +sequence, which is the privilege used by the adapter's implicit `nextval`; sequence `SELECT` is +not required because no adapter operation reads sequence state. + +The Docker fixture includes a writer role with complete table/hash-column authority but no +sequence grant. Its health is deterministically unhealthy and a new-key upsert fails. Granting +only sequence `USAGE` makes health green and the same port upsert succeeds. Sequence discovery is +guarded for partial schemas so a missing `id` column produces the existing sanitized schema +diagnostic instead of a PostgreSQL error. + +Fresh verification for this follow-up: + +- Docker pgvector L0 after formatting: `17 passed`. +- Expanded focused adapter/config/parity suite: `57 passed`. +- Full harness: `467 passed, 5 deselected`. +- Changed-file Ruff lint/format and `git diff --check`: clean. diff --git a/harness/tests/l0/test_pgvector_store.py b/harness/tests/l0/test_pgvector_store.py index 2181b540..4dea38ea 100644 --- a/harness/tests/l0/test_pgvector_store.py +++ b/harness/tests/l0/test_pgvector_store.py @@ -1,4 +1,5 @@ import pytest +from psycopg2.errors import InsufficientPrivilege from sqlalchemy import create_engine, text from sqlalchemy.exc import ProgrammingError from testcontainers.postgres import PostgresContainer @@ -61,7 +62,11 @@ def vector_configs(): connection.exec_driver_sql("CREATE ROLE vector_l0_reader LOGIN PASSWORD 'reader'") connection.exec_driver_sql("CREATE ROLE vector_l0_writer LOGIN PASSWORD 'writer'") connection.exec_driver_sql( - "GRANT USAGE ON SCHEMA vectors TO vector_l0_reader, vector_l0_writer" + "CREATE ROLE vector_l0_no_sequence LOGIN PASSWORD 'no_sequence'" + ) + connection.exec_driver_sql( + "GRANT USAGE ON SCHEMA vectors TO vector_l0_reader, vector_l0_writer, " + "vector_l0_no_sequence" ) connection.exec_driver_sql( "GRANT SELECT ON ALL TABLES IN SCHEMA vectors TO vector_l0_reader" @@ -71,11 +76,12 @@ def vector_configs(): ) for table in ("schema_records", "evidence", "memory"): connection.exec_driver_sql( - f"GRANT INSERT, UPDATE ON vectors.{table} TO vector_l0_writer" + f"GRANT INSERT, UPDATE ON vectors.{table} " + "TO vector_l0_writer, vector_l0_no_sequence" ) connection.exec_driver_sql( f"GRANT SELECT (record_key, kind, content_hash) " - f"ON vectors.{table} TO vector_l0_writer" + f"ON vectors.{table} TO vector_l0_writer, vector_l0_no_sequence" ) engine.dispose() reader_config = admin_config.model_copy( @@ -84,14 +90,17 @@ def vector_configs(): writer_config = admin_config.model_copy( update={"user": "vector_l0_writer", "password": "writer"} ) - yield admin_config, reader_config, writer_config + no_sequence_config = admin_config.model_copy( + update={"user": "vector_l0_no_sequence", "password": "no_sequence"} + ) + yield admin_config, reader_config, writer_config, no_sequence_config @pytest.fixture def store(vector_configs): from tht.adapters.vector.pgvector import PgVectorStore - _, reader_config, writer_config = vector_configs + _, reader_config, writer_config, _ = vector_configs store = PgVectorStore(reader_config, writer_config, expected_dimension=2) store.upsert("memory", [_record("reset", [0.0, 1.0])]) yield store @@ -179,7 +188,7 @@ def test_pgvector_rejects_kinds_not_belonging_to_collection(store): def test_pgvector_separates_read_and_write_credentials(vector_configs): from tht.adapters.vector.pgvector import PgVectorStore - _, reader_config, writer_config = vector_configs + _, reader_config, writer_config, _ = vector_configs reader = PgVectorStore(reader_config, expected_dimension=2) assert reader.capabilities.search is True assert reader.capabilities.upsert is False @@ -194,7 +203,7 @@ def test_pgvector_separates_read_and_write_credentials(vector_configs): def test_pgvector_database_roles_are_least_privilege(vector_configs): - _, reader_config, writer_config = vector_configs + _, reader_config, writer_config, _ = vector_configs reader_engine = create_engine( f"postgresql+psycopg2://{reader_config.user}:{reader_config.password}" f"@{reader_config.host}:{reader_config.port}/{reader_config.database}" @@ -224,10 +233,39 @@ def test_pgvector_database_roles_are_least_privilege(vector_configs): writer_engine.dispose() +def test_pgvector_writer_health_requires_sequence_usage(vector_configs): + from tht.adapters.vector.pgvector import PgVectorStore + + admin_config, _, _, no_sequence_config = vector_configs + store = PgVectorStore(None, no_sequence_config, expected_dimension=2) + + health = store.health() + assert health.ok is False + assert health.write_reachable is False + assert health.write_detail == ( + "vector schema incomplete: missing sequence privileges evidence, memory, schema_records" + ) + with pytest.raises(InsufficientPrivilege): + store.upsert("memory", [_record("needs-sequence", [1.0, 0.0])]) + + admin_engine = create_engine( + f"postgresql+psycopg2://{admin_config.user}:{admin_config.password}" + f"@{admin_config.host}:{admin_config.port}/{admin_config.database}" + ) + with admin_engine.begin() as connection: + connection.exec_driver_sql( + "GRANT USAGE ON ALL SEQUENCES IN SCHEMA vectors TO vector_l0_no_sequence" + ) + admin_engine.dispose() + + assert store.health().ok is True + assert store.upsert("memory", [_record("has-sequence", [1.0, 0.0])]) == 1 + + def test_pgvector_health_reports_dimension_and_each_connection(vector_configs): from tht.adapters.vector.pgvector import PgVectorStore - _, reader_config, writer_config = vector_configs + _, reader_config, writer_config, _ = vector_configs health = PgVectorStore(reader_config, writer_config, expected_dimension=2).health() assert health.ok is True assert health.read_reachable is True @@ -247,7 +285,7 @@ def test_pgvector_health_reports_dimension_and_each_connection(vector_configs): def test_pgvector_health_rejects_clean_and_partial_schemas(vector_configs): from tht.adapters.vector.pgvector import PgVectorStore - admin_config, _, _ = vector_configs + admin_config, _, _, _ = vector_configs engine = create_engine( f"postgresql+psycopg2://{admin_config.user}:{admin_config.password}" f"@{admin_config.host}:{admin_config.port}/{admin_config.database}" diff --git a/harness/tht/adapters/vector/pgvector.py b/harness/tht/adapters/vector/pgvector.py index c9a009c0..fd975da3 100644 --- a/harness/tht/adapters/vector/pgvector.py +++ b/harness/tht/adapters/vector/pgvector.py @@ -98,11 +98,27 @@ class PgVectorStore: AND has_column_privilege( current_user, c.oid, 'content_hash', 'SELECT' ) - AND has_column_privilege(current_user, c.oid, 'kind', 'SELECT') + AND has_column_privilege(current_user, c.oid, 'kind', 'SELECT'), + CASE WHEN id_attr.attname IS NOT NULL THEN + pg_get_serial_sequence( + format('%%I.%%I', n.nspname, c.relname), 'id' + ) + END AS id_sequence, + CASE WHEN id_attr.attname IS NOT NULL THEN + has_sequence_privilege( + current_user, + pg_get_serial_sequence( + format('%%I.%%I', n.nspname, c.relname), 'id' + ), + 'USAGE' + ) + END AS sequence_usage FROM pg_class c JOIN pg_namespace n ON n.oid = c.relnamespace LEFT JOIN pg_attribute a ON a.attrelid = c.oid AND a.attname = 'embedding' AND NOT a.attisdropped + LEFT JOIN pg_attribute id_attr ON id_attr.attrelid = c.oid + AND id_attr.attname = 'id' AND NOT id_attr.attisdropped WHERE n.nspname = %s AND c.relname = ANY(%s) AND c.relkind IN ('r', 'p')""", (self._schema, list(ALLOWED_COLLECTIONS)), @@ -117,6 +133,12 @@ class PgVectorStore: if (writable and not (row[3] and row[4] and row[5])) or (not writable and not row[2]) ) + missing_sequences = sorted( + row[0] for row in rows if writable and row[6] is None + ) + sequence_privilege_missing = sorted( + row[0] for row in rows if writable and row[6] is not None and not row[7] + ) problems = [] if missing_tables: problems.append("missing tables " + ", ".join(missing_tables)) @@ -129,6 +151,12 @@ class PgVectorStore: problems.append( f"missing {authority} privileges " + ", ".join(privilege_missing) ) + if missing_sequences: + problems.append("missing id sequences " + ", ".join(missing_sequences)) + if sequence_privilege_missing: + problems.append( + "missing sequence privileges " + ", ".join(sequence_privilege_missing) + ) if problems: return False, "vector schema incomplete: " + "; ".join(problems), set() dimensions = {