fix(vector): verify writer sequence privileges
This commit is contained in:
@@ -73,3 +73,23 @@ Fresh verification after the fix wave:
|
|||||||
- Expanded focused adapter/config suite: `56 passed`.
|
- Expanded focused adapter/config suite: `56 passed`.
|
||||||
- Full harness: `466 passed, 5 deselected`.
|
- Full harness: `466 passed, 5 deselected`.
|
||||||
- Changed-file Ruff lint/format and `git diff --check`: clean.
|
- 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.
|
||||||
|
|||||||
@@ -1,4 +1,5 @@
|
|||||||
import pytest
|
import pytest
|
||||||
|
from psycopg2.errors import InsufficientPrivilege
|
||||||
from sqlalchemy import create_engine, text
|
from sqlalchemy import create_engine, text
|
||||||
from sqlalchemy.exc import ProgrammingError
|
from sqlalchemy.exc import ProgrammingError
|
||||||
from testcontainers.postgres import PostgresContainer
|
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_reader LOGIN PASSWORD 'reader'")
|
||||||
connection.exec_driver_sql("CREATE ROLE vector_l0_writer LOGIN PASSWORD 'writer'")
|
connection.exec_driver_sql("CREATE ROLE vector_l0_writer LOGIN PASSWORD 'writer'")
|
||||||
connection.exec_driver_sql(
|
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(
|
connection.exec_driver_sql(
|
||||||
"GRANT SELECT ON ALL TABLES IN SCHEMA vectors TO vector_l0_reader"
|
"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"):
|
for table in ("schema_records", "evidence", "memory"):
|
||||||
connection.exec_driver_sql(
|
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(
|
connection.exec_driver_sql(
|
||||||
f"GRANT SELECT (record_key, kind, content_hash) "
|
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()
|
engine.dispose()
|
||||||
reader_config = admin_config.model_copy(
|
reader_config = admin_config.model_copy(
|
||||||
@@ -84,14 +90,17 @@ def vector_configs():
|
|||||||
writer_config = admin_config.model_copy(
|
writer_config = admin_config.model_copy(
|
||||||
update={"user": "vector_l0_writer", "password": "writer"}
|
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
|
@pytest.fixture
|
||||||
def store(vector_configs):
|
def store(vector_configs):
|
||||||
from tht.adapters.vector.pgvector import PgVectorStore
|
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 = PgVectorStore(reader_config, writer_config, expected_dimension=2)
|
||||||
store.upsert("memory", [_record("reset", [0.0, 1.0])])
|
store.upsert("memory", [_record("reset", [0.0, 1.0])])
|
||||||
yield store
|
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):
|
def test_pgvector_separates_read_and_write_credentials(vector_configs):
|
||||||
from tht.adapters.vector.pgvector import PgVectorStore
|
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)
|
reader = PgVectorStore(reader_config, expected_dimension=2)
|
||||||
assert reader.capabilities.search is True
|
assert reader.capabilities.search is True
|
||||||
assert reader.capabilities.upsert is False
|
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):
|
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(
|
reader_engine = create_engine(
|
||||||
f"postgresql+psycopg2://{reader_config.user}:{reader_config.password}"
|
f"postgresql+psycopg2://{reader_config.user}:{reader_config.password}"
|
||||||
f"@{reader_config.host}:{reader_config.port}/{reader_config.database}"
|
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()
|
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):
|
def test_pgvector_health_reports_dimension_and_each_connection(vector_configs):
|
||||||
from tht.adapters.vector.pgvector import PgVectorStore
|
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()
|
health = PgVectorStore(reader_config, writer_config, expected_dimension=2).health()
|
||||||
assert health.ok is True
|
assert health.ok is True
|
||||||
assert health.read_reachable 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):
|
def test_pgvector_health_rejects_clean_and_partial_schemas(vector_configs):
|
||||||
from tht.adapters.vector.pgvector import PgVectorStore
|
from tht.adapters.vector.pgvector import PgVectorStore
|
||||||
|
|
||||||
admin_config, _, _ = vector_configs
|
admin_config, _, _, _ = vector_configs
|
||||||
engine = create_engine(
|
engine = create_engine(
|
||||||
f"postgresql+psycopg2://{admin_config.user}:{admin_config.password}"
|
f"postgresql+psycopg2://{admin_config.user}:{admin_config.password}"
|
||||||
f"@{admin_config.host}:{admin_config.port}/{admin_config.database}"
|
f"@{admin_config.host}:{admin_config.port}/{admin_config.database}"
|
||||||
|
|||||||
@@ -98,11 +98,27 @@ class PgVectorStore:
|
|||||||
AND has_column_privilege(
|
AND has_column_privilege(
|
||||||
current_user, c.oid, 'content_hash', 'SELECT'
|
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
|
FROM pg_class c
|
||||||
JOIN pg_namespace n ON n.oid = c.relnamespace
|
JOIN pg_namespace n ON n.oid = c.relnamespace
|
||||||
LEFT JOIN pg_attribute a ON a.attrelid = c.oid
|
LEFT JOIN pg_attribute a ON a.attrelid = c.oid
|
||||||
AND a.attname = 'embedding' AND NOT a.attisdropped
|
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)
|
WHERE n.nspname = %s AND c.relname = ANY(%s)
|
||||||
AND c.relkind IN ('r', 'p')""",
|
AND c.relkind IN ('r', 'p')""",
|
||||||
(self._schema, list(ALLOWED_COLLECTIONS)),
|
(self._schema, list(ALLOWED_COLLECTIONS)),
|
||||||
@@ -117,6 +133,12 @@ class PgVectorStore:
|
|||||||
if (writable and not (row[3] and row[4] and row[5]))
|
if (writable and not (row[3] and row[4] and row[5]))
|
||||||
or (not writable and not row[2])
|
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 = []
|
problems = []
|
||||||
if missing_tables:
|
if missing_tables:
|
||||||
problems.append("missing tables " + ", ".join(missing_tables))
|
problems.append("missing tables " + ", ".join(missing_tables))
|
||||||
@@ -129,6 +151,12 @@ class PgVectorStore:
|
|||||||
problems.append(
|
problems.append(
|
||||||
f"missing {authority} privileges " + ", ".join(privilege_missing)
|
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:
|
if problems:
|
||||||
return False, "vector schema incomplete: " + "; ".join(problems), set()
|
return False, "vector schema incomplete: " + "; ".join(problems), set()
|
||||||
dimensions = {
|
dimensions = {
|
||||||
|
|||||||
Reference in New Issue
Block a user