From b93bd0a3528bdcaf4d415c7baf4ca65ccf398aed Mon Sep 17 00:00:00 2001 From: warheart1984-ctrl Date: Fri, 16 Oct 2026 18:34:02 -0400 Subject: [PATCH 1/4] Phase 1 slice 1: fixes for the CI run and Codex round 2 Tests that manage their own schemas now drop the v10 control schema twin (a second run on the same server failed with 'schema already exists'); key issuance reads the tenant row FOR SHARE so it cannot race a deletion; --expires-days 0 means 'expires now'; the rollback SQL quotes the control schema (v10_rollback_sql); a repeated 'deleted' keeps the first deleted_at. Co-Authored-By: Claude Sonnet 5.5 --- app/pg_schema.py | 9 ++- app/tenant_keys.py | 6 +- docs/tenant_keys.md | 2 +- tests/test_block_anchors.py | 1 + tests/test_ledger_search.py | 4 +- tests/test_pg_replay.py | 1 + tests/test_tenant_keys.py | 122 +++++++++++++++++++++++++++++++++++- 7 files changed, 136 insertions(+), 9 deletions(-) diff --git a/app/pg_schema.py b/app/pg_schema.py index 6903b93..80a4c0f 100644 --- a/app/pg_schema.py +++ b/app/pg_schema.py @@ -920,7 +920,8 @@ def _v6(skip: frozenset[str] | set[str] = frozenset()) -> str: CREATE TRIGGER tenants_lifecycle BEFORE UPDATE ON __CONTROL__.tenants FOR EACH ROW EXECUTE FUNCTION __CONTROL__.tenant_lifecycle_guard(); CREATE FUNCTION __CONTROL__.key_for_live_tenant_only() RETURNS trigger LANGUAGE plpgsql SET search_path = __CONTROL__, pg_temp AS $fn$ BEGIN - IF (SELECT status FROM tenants WHERE tenant_key = NEW.tenant_key) IN ('deleting', 'deleted') THEN + -- FOR SHARE: a concurrent transition to deleting/deleted waits for this insert to commit (and then revokes the new key); one that got there first is seen here. No window in between. + IF (SELECT status FROM tenants WHERE tenant_key = NEW.tenant_key FOR SHARE) IN ('deleting', 'deleted') THEN RAISE EXCEPTION 'no key can be issued for a deleting or deleted tenant' USING ERRCODE = 'check_violation'; END IF; RETURN NEW; @@ -937,6 +938,12 @@ def _v6(skip: frozenset[str] | set[str] = frozenset()) -> str: DELETE FROM schema_version WHERE version = 10; """ +def v10_rollback_sql(schema: str) -> str: + """V10_ROLLBACK for a ledger schema, with the control schema's name QUOTED (a mixed-case or unusual name is a quoted identifier, as v10 created it).""" + quoted = '"' + control_schema_name(schema).replace('"', '""') + '"' + return V10_ROLLBACK.replace("__CONTROL__", quoted) + + MIGRATIONS: list[tuple[int, str]] = [(1, _V1), (2, _V2), (3, _V3), (4, _V4), (5, _V5), (6, _V6), (7, _V7), (8, _V8), (9, _V9), (10, _V10)] EXPECTED_SCHEMA_VERSION = MIGRATIONS[-1][0] diff --git a/app/tenant_keys.py b/app/tenant_keys.py index b1b6b18..6267f47 100644 --- a/app/tenant_keys.py +++ b/app/tenant_keys.py @@ -149,7 +149,7 @@ def set_tenant_status(conn: psycopg.Connection, control: str, tenant_key: str, s if current is not None and (current["status"] if isinstance(current, dict) else current[0]) == "deleted" and status != "deleted": raise ValueError("a deleted tenant cannot be reactivated") # (the database refuses it too: tenants_lifecycle trigger) cur = conn.execute( - sql.SQL("UPDATE {}.tenants SET status = %s, deleted_at = CASE WHEN %s = 'deleted' THEN now() ELSE deleted_at END WHERE tenant_key = %s").format(sql.Identifier(control)), + sql.SQL("UPDATE {}.tenants SET status = %s, deleted_at = CASE WHEN %s = 'deleted' THEN coalesce(deleted_at, now()) ELSE deleted_at END WHERE tenant_key = %s").format(sql.Identifier(control)), (status, status, tenant_key)) return cur.rowcount == 1 @@ -204,7 +204,9 @@ def main(argv: list[str] | None = None) -> int: print(create_tenant(conn, control, args.label)) elif args.cmd == "create-key": scopes = tuple(s.strip() for s in args.scopes.split(",") if s.strip()) - expires = datetime.now(timezone.utc) + timedelta(days=args.expires_days) if args.expires_days else None + if args.expires_days is not None and args.expires_days < 0: + raise ValueError("--expires-days cannot be negative") + expires = datetime.now(timezone.utc) + timedelta(days=args.expires_days) if args.expires_days is not None else None # 0 means "expires now", not "never" token = create_key(conn, control, args.tenant, pepper(), label=args.label, scopes=scopes, expires_at=expires) if args.token_file: fd = os.open(args.token_file, os.O_WRONLY | os.O_CREAT | os.O_EXCL, 0o600) diff --git a/docs/tenant_keys.md b/docs/tenant_keys.md index b8a4c98..e7e08a8 100644 --- a/docs/tenant_keys.md +++ b/docs/tenant_keys.md @@ -23,7 +23,7 @@ and entering `deleting` or `deleted` revokes every key of the tenant; a second t revoke its keys. The ledger schema may be up to 63 characters: past 55 the control schema name is shortened deterministically (a prefix, a hash of the whole name, then `_control`) so it never exceeds PostgreSQL's limit and two different long schemas never share one. -Rollback: `V10_ROLLBACK` (with `__CONTROL__` replaced by the control schema's name) drops the control schema and the version row. It holds no ledger data, so no ledger data changes either way. As with v8 and v9, a v9 +Rollback: `pg_schema.v10_rollback_sql(schema)` (`V10_ROLLBACK` with the control schema's name, quoted as v10 created it) drops the control schema and the version row. It holds no ledger data, so no ledger data changes either way. As with v8 and v9, a v9 application refuses a v10 database and the reverse; rehearse on a throwaway stack before the live migration. ## Token and verification diff --git a/tests/test_block_anchors.py b/tests/test_block_anchors.py index badf622..2003b07 100644 --- a/tests/test_block_anchors.py +++ b/tests/test_block_anchors.py @@ -285,3 +285,4 @@ def test_the_dump_reader_and_the_live_reader_agree_including_blocks(tmp_path): finally: with psycopg.connect(dsn, autocommit=True) as conn: conn.execute("DROP SCHEMA jarvis CASCADE") + conn.execute("DROP SCHEMA IF EXISTS jarvis_control CASCADE") # v10's control schema twin: left behind it would break the next run on the same server diff --git a/tests/test_ledger_search.py b/tests/test_ledger_search.py index 8004690..3590e3d 100644 --- a/tests/test_ledger_search.py +++ b/tests/test_ledger_search.py @@ -18,7 +18,7 @@ from app.ledger_search import SearchParams, record_tokens, search_ledger, tokens from app.main import app from app.models import EvidenceLink, MemoryCreate -from app.pg_schema import EXPECTED_SCHEMA_VERSION, V9_ROLLBACK, V10_ROLLBACK, control_schema_name, migrate +from app.pg_schema import EXPECTED_SCHEMA_VERSION, V9_ROLLBACK, migrate, v10_rollback_sql from app.pg_store import PostgresRowStore from app.store import JarvisStore @@ -361,7 +361,7 @@ def test_v9_on_a_populated_ledger_changes_no_row_hash_or_root_and_rolls_back_cle pg_store.close_pools() with psycopg.connect(pg_schema.admin_dsn, options=f"-c search_path={pg_schema.schema}") as conn: # one transaction - conn.execute(V10_ROLLBACK.replace("__CONTROL__", control_schema_name(pg_schema.schema))) # newer migrations first (v10 added a control schema) + conn.execute(v10_rollback_sql(pg_schema.schema)) # newer migrations first (v10 added a control schema) conn.execute(V9_ROLLBACK) with pg_schema.admin_conn() as conn: assert conn.execute("SELECT max(version) FROM schema_version").fetchone()[0] == 8 diff --git a/tests/test_pg_replay.py b/tests/test_pg_replay.py index 6cde4eb..4fac285 100644 --- a/tests/test_pg_replay.py +++ b/tests/test_pg_replay.py @@ -163,6 +163,7 @@ def test_a_restored_copy_replays_to_the_identical_state(pg, store): finally: pg_store.close_pools() conn.execute('DROP SCHEMA "restored" CASCADE') + conn.execute('DROP SCHEMA IF EXISTS "restored_control" CASCADE') # v10's control schema twin: left behind it would break the next run on the same server def test_deleted_records_are_not_in_the_state_and_are_counted(pg, store): diff --git a/tests/test_tenant_keys.py b/tests/test_tenant_keys.py index 2fa6437..8a3dee1 100644 --- a/tests/test_tenant_keys.py +++ b/tests/test_tenant_keys.py @@ -15,7 +15,7 @@ from psycopg.rows import dict_row from app import pg_store, tenant_keys -from app.pg_schema import V10_ROLLBACK, control_schema_name, migrate +from app.pg_schema import control_schema_name, migrate, v10_rollback_sql pg_only = pytest.mark.postgres PEPPER = b"unit-test-pepper-0123456789-0123456789" @@ -272,7 +272,7 @@ def test_v10_rolls_back_cleanly_and_forward_again_and_changes_no_ledger_data(pg_ control = control_schema_name(pg_schema.schema) assert migrate(pg_schema.admin_dsn, schema=pg_schema.schema, app_role="jarvis_app_test") == 10 with psycopg.connect(pg_schema.admin_dsn, options=f"-c search_path={pg_schema.schema}") as conn: - conn.execute(V10_ROLLBACK.replace("__CONTROL__", control)) + conn.execute(v10_rollback_sql(pg_schema.schema)) with pg_schema.admin_conn() as conn: assert conn.execute("SELECT max(version) FROM schema_version").fetchone()[0] == 9 assert conn.execute("SELECT to_regnamespace(%s) IS NULL", (control,)).fetchone()[0] @@ -403,9 +403,125 @@ def checksums(): before = checksums() assert all(before.values()) with psycopg.connect(pg_schema.admin_dsn, options=f"-c search_path={pg_schema.schema}") as conn: - conn.execute(V10_ROLLBACK.replace("__CONTROL__", control)) + conn.execute(v10_rollback_sql(pg_schema.schema)) assert checksums() == before assert migrate(pg_schema.admin_dsn, schema=pg_schema.schema, app_role="jarvis_app_test") == 10 assert checksums() == before with pg_schema.admin_conn() as conn: assert conn.execute("SELECT count(*) FROM memories WHERE tenant_key = %s", (tenant,)).fetchone()[0] == 3 # the key tenant's rows survived (unreachable without v10, intact) + + +# ----------------------------------------------------------------------------------------------------- review findings (Codex, round 2 on #107) +@pg_only +def test_issuing_a_key_and_deleting_the_tenant_cannot_race_either_way_round(pg): + """The trigger reads the tenant's status FOR SHARE: a concurrent transition to deleting waits for an uncommitted insert (and then revokes that key), and an insert waits for an uncommitted + transition (and then sees it). Neither order can leave an active key on a deleting tenant.""" + import threading + + ident = psycopg.sql.Identifier(pg.control) + tenant, _ = _issue(pg) + open_conns = [] + + def conn(autocommit=False): + c = psycopg.connect(pg.admin_dsn, autocommit=autocommit, row_factory=dict_row, options="-c lock_timeout=20000") + open_conns.append(c) + return c + + try: + # order 1: the transition holds the row; the insert must wait, then be refused + deleting = conn() + deleting.execute(psycopg.sql.SQL("UPDATE {}.tenants SET status = 'deleting' WHERE tenant_key = %s").format(ident), (tenant,)) # uncommitted + outcome = {} + + def insert_while_deleting(): + c = conn(autocommit=True) + try: + tenant_keys.create_key(c, pg.control, tenant, PEPPER) + outcome["insert"] = "issued" + except psycopg.errors.CheckViolation: + outcome["insert"] = "refused" + + t = threading.Thread(target=insert_while_deleting) + t.start() + t.join(timeout=1.0) + still_waiting = t.is_alive() + deleting.commit() + t.join(timeout=25) + assert still_waiting, "the insert should be waiting for the uncommitted transition" + assert outcome["insert"] == "refused" + # order 2: the insert holds the share lock; the transition must wait, then revoke the new key + tenant2, _ = _issue(pg) + inserting = conn() + new_token = tenant_keys.create_key(inserting, pg.control, tenant2, PEPPER) # uncommitted + + def delete_while_inserting(): + tenant_keys.set_tenant_status(conn(autocommit=True), pg.control, tenant2, "deleting") + + t2 = threading.Thread(target=delete_while_inserting) + t2.start() + t2.join(timeout=1.0) + still_waiting = t2.is_alive() + inserting.commit() + t2.join(timeout=25) + assert still_waiting, "the transition should be waiting for the uncommitted insert" + with _app(pg) as c: + assert tenant_keys.resolve(c, pg.control, new_token, PEPPER) is None # the key that raced in was revoked + finally: + for c in open_conns: # never leave a transaction open: it would block the fixture's teardown + try: + c.rollback() + except Exception: # noqa: BLE001 + pass + c.close() + + +@pg_only +def test_an_explicit_zero_day_expiry_expires_now_and_a_negative_one_is_refused(pg, monkeypatch, tmp_path, capsys): + monkeypatch.setenv("JARVIS_DATABASE_MIGRATE_URL", pg.admin_dsn) + monkeypatch.setenv("JARVIS_DATABASE_SCHEMA", pg.schema) + monkeypatch.setenv("JARVIS_KEY_PEPPER", PEPPER.decode()) + assert tenant_keys.main(["create-tenant"]) == 0 + tenant = capsys.readouterr().out.strip() + f = tmp_path / "t0" + assert tenant_keys.main(["create-key", "--tenant", tenant, "--expires-days", "0", "--token-file", str(f)]) == 0 + with _app(pg) as c, _admin(pg) as a: + assert tenant_keys.list_keys(a, pg.control, tenant)[0]["expires_at"] is not None # 0 is an expiry (now), not "never" + assert tenant_keys.resolve(c, pg.control, f.read_text().strip(), PEPPER) is None + assert tenant_keys.main(["create-key", "--tenant", tenant, "--expires-days", "-1", "--token-file", str(tmp_path / "t1")]) == 1 + assert tenant_keys.main(["create-key", "--tenant", tenant, "--token-file", str(tmp_path / "t2")]) == 0 + with _admin(pg) as a: + assert any(k["expires_at"] is None for k in tenant_keys.list_keys(a, pg.control, tenant)) # omitting it still means no expiry + + +@pg_only +def test_setting_deleted_again_keeps_the_first_deletion_timestamp(pg): + tenant, _ = _issue(pg) + with _admin(pg) as a: + tenant_keys.set_tenant_status(a, pg.control, tenant, "deleted") + first = tenant_keys.list_tenants(a, pg.control)[0]["deleted_at"] + assert first is not None + tenant_keys.set_tenant_status(a, pg.control, tenant, "deleted") + assert tenant_keys.list_tenants(a, pg.control)[0]["deleted_at"] == first + + +@pytest.mark.postgres +def test_v10_and_its_rollback_work_for_a_mixed_case_ledger_schema(pg_server): + """validate_schema_name allows uppercase; v10 creates the control schema as a quoted identifier, so the rollback must quote it too.""" + admin_dsn, _ = pg_server + schema = "Ledger_MixedCase" + with psycopg.connect(admin_dsn, autocommit=True) as conn: + conn.execute(psycopg.sql.SQL("CREATE SCHEMA {}").format(psycopg.sql.Identifier(schema))) + try: + assert migrate(admin_dsn, schema=schema, app_role="jarvis_app_test") == 10 + with psycopg.connect(admin_dsn, autocommit=True) as conn: + assert conn.execute("SELECT count(*) FROM pg_namespace WHERE nspname = 'Ledger_MixedCase_control'").fetchone()[0] == 1 + with psycopg.connect(admin_dsn) as conn: # (one transaction; the mixed-case schema is a quoted identifier) + conn.execute(psycopg.sql.SQL("SET LOCAL search_path TO {}").format(psycopg.sql.Identifier(schema))) + conn.execute(v10_rollback_sql(schema)) + with psycopg.connect(admin_dsn, autocommit=True) as conn: + assert conn.execute("SELECT count(*) FROM pg_namespace WHERE nspname = 'Ledger_MixedCase_control'").fetchone()[0] == 0 + assert conn.execute(psycopg.sql.SQL("SELECT max(version) FROM {}.schema_version").format(psycopg.sql.Identifier(schema))).fetchone()[0] == 9 + finally: + with psycopg.connect(admin_dsn, autocommit=True) as conn: + conn.execute(psycopg.sql.SQL("DROP SCHEMA IF EXISTS {} CASCADE").format(psycopg.sql.Identifier(schema))) + conn.execute(psycopg.sql.SQL("DROP SCHEMA IF EXISTS {} CASCADE").format(psycopg.sql.Identifier(control_schema_name(schema)))) From 5b6f4b5a0a3bfff0451675a2dcf6b7a8f56a087d Mon Sep 17 00:00:00 2001 From: warheart1984-ctrl Date: Fri, 16 Oct 2026 18:44:21 -0400 Subject: [PATCH 2/4] RFC appendix: requirements from the mcp-gateway and mcp-zero studies Co-Authored-By: Claude Sonnet 5.5 --- docs/RFC-multi-tenant-ledger.md | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/docs/RFC-multi-tenant-ledger.md b/docs/RFC-multi-tenant-ledger.md index 95d7707..fabd72a 100644 --- a/docs/RFC-multi-tenant-ledger.md +++ b/docs/RFC-multi-tenant-ledger.md @@ -376,6 +376,25 @@ epoch as a Merkle root, items 1, 6, 10 and 11; deletion against retained backups altered**, and a **deleted tenant's bundle replayed** must be **rejected before any row changes** (and the attempt recorded); an **epoch or event sequence rolled back by a restore** is rejected unless a matching recorded event explains it. Detection after the fact does not pass the gate for the rejected cases. +### From two open-source gateway studies (2026-10-16) + +Studies of `Shcherbin96/mcp-gateway` (tenant-scoped approvals and an append-only audit, with isolation by hand-written query filters) and `abwaters/mcp-zero` (user and group policy, fail-closed masking, payload-free audit; no tenant +concept at all) confirmed the direction and added three requirements. Both are repository claims plus their own test runs, not something verified on this system. + +16. **Every process-level cache and limiter is keyed by the full security scope (sections 4 and 8).** mcp-zero's fix was to key pooled transports and token caches by the authenticated subject. Audit of this + codebase (2026-10-16): the embedding caches (`app/amul_rag.py` `_EMBED_CACHE`, `app/emr_embed.py`) map content to a vector and are only read for records the caller was already given, so they do not cross tenants; + neither has a size bound. The rule going forward: a new cache that can return anything derived from stored data must include the tenant key in its key, and a Red Team test must show two tenants storing the + same text do not see each other's cache effect. +17. **Rate limiting is per credential, not per address (section 3).** `McpRateLimiter` (`app/public_security.py`) keys on the client address, only on public deployments and only on `/mcp`, is process-local, and never evicts a + bucket. Behind a proxy that hides the client address, tenants share one bucket. Phase 1: key by the authenticated principal (tenant key plus credential id) when there is one, fall back to the address when there + is not, and evict idle buckets. An availability gap, not an isolation one. +18. **Decision transitions are conditional and carry the tenant (section 4).** mcp-gateway decides an approval with `WHERE id = $1 AND status = 'pending' AND tenant_id = $2`, so a decision is idempotent and a guessed id + from another tenant matches nothing. Our human-decision and draft/verified paths already run under forced row-level security; add an explicit test that a foreign tenant's decision on a guessed id changes nothing + and a repeated decision is a no-op. + +Also noted from mcp-gateway: its role lookup joined on the agent id without asserting the tenant, relying on globally unique ids and the issuer. Our key resolution takes the tenant from the control-schema row of +the presented key and never from a request value, which is the stronger rule both studies recommend. + ### Exit criteria (section 9) 1 (Red Team), 3, 4 and 5 are open. 6 is met (matrix in CI, fail-closed on unregistered routes). **2 is only partly met** and stays open: in strict mode the app refuses to start with the From 54d1c6ce5bf48a4b3234da37283566b240badfb7 Mon Sep 17 00:00:00 2001 From: warheart1984-ctrl Date: Fri, 16 Oct 2026 18:48:00 -0400 Subject: [PATCH 3/4] RFC appendix: items from the mcp-multitenant-gateway study Co-Authored-By: Claude Sonnet 5.5 --- docs/RFC-multi-tenant-ledger.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/docs/RFC-multi-tenant-ledger.md b/docs/RFC-multi-tenant-ledger.md index fabd72a..6e190df 100644 --- a/docs/RFC-multi-tenant-ledger.md +++ b/docs/RFC-multi-tenant-ledger.md @@ -388,10 +388,20 @@ concept at all) confirmed the direction and added three requirements. Both are r 17. **Rate limiting is per credential, not per address (section 3).** `McpRateLimiter` (`app/public_security.py`) keys on the client address, only on public deployments and only on `/mcp`, is process-local, and never evicts a bucket. Behind a proxy that hides the client address, tenants share one bucket. Phase 1: key by the authenticated principal (tenant key plus credential id) when there is one, fall back to the address when there is not, and evict idle buckets. An availability gap, not an isolation one. + From `skunkworks-powerweave/mcp-multitenant-gateway` (third study, 2026-10-16): its limiter is keyed by the bearer key prefix and runs *before* verification, so brute-force guessing of one key is throttled too, + and **it refuses to start with more than one worker unless a shared limiter backend is declared**, so a process-local limit cannot silently multiply per replica. Adopt both: throttle by the public `key_id` + before the HMAC check, and make the process-local limiter a startup refusal when more than one worker or replica is configured without a shared backend. 18. **Decision transitions are conditional and carry the tenant (section 4).** mcp-gateway decides an approval with `WHERE id = $1 AND status = 'pending' AND tenant_id = $2`, so a decision is idempotent and a guessed id from another tenant matches nothing. Our human-decision and draft/verified paths already run under forced row-level security; add an explicit test that a foreign tenant's decision on a guessed id changes nothing and a repeated decision is a no-op. +19. **A forged tenant field never widens scope (section 8, attack class 11).** The third study's whole isolation rule is that the authenticated tenant overrides any client-supplied `tenant_id`. We go further: no + route or tool takes a tenant from the request at all (the tenant is `Principal.tenant_key`, from the key row or the OAuth identity). Nothing yet *pins* that. Add to the matrix, for every case and for both + OAuth and key tenants, a variant that sends `tenant_id`, `_tenant_id` and `tenant_key` naming another tenant in the body, the query string and a header; the response must be byte-identical to the request without + them (ignored, or rejected the same way for everybody), and a route that starts reading such a field fails the test. +20. **Candidate, not scheduled: idempotency keys on writes.** A retried `remember` (a timeout, a client retry) can store a duplicate draft. An optional caller-supplied idempotency key, scoped to the tenant and + credential, would make a retry return the first result. This is a feature, not an isolation fix; record it, decide later. + Also noted from mcp-gateway: its role lookup joined on the agent id without asserting the tenant, relying on globally unique ids and the issuer. Our key resolution takes the tenant from the control-schema row of the presented key and never from a request value, which is the stronger rule both studies recommend. From b680f05d2d919fa8453e55dfdc0b43b797adcd1e Mon Sep 17 00:00:00 2001 From: warheart1984-ctrl Date: Fri, 16 Oct 2026 18:51:05 -0400 Subject: [PATCH 4/4] v10 rollback SQL: qualify schema_version with the ledger schema (Codex round 3) Co-Authored-By: Claude Sonnet 5.5 --- app/pg_schema.py | 8 +++++--- tests/test_tenant_keys.py | 13 +++++++++++++ 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/app/pg_schema.py b/app/pg_schema.py index 80a4c0f..c98fead 100644 --- a/app/pg_schema.py +++ b/app/pg_schema.py @@ -935,13 +935,15 @@ def _v6(skip: frozenset[str] | set[str] = frozenset()) -> str: # Rolling V10 back removes only what V10 added (run as the migration role, inside one transaction). The control schema holds no ledger data. V10_ROLLBACK = """ DROP SCHEMA IF EXISTS __CONTROL__ CASCADE; -DELETE FROM schema_version WHERE version = 10; +DELETE FROM __LEDGER__.schema_version WHERE version = 10; """ def v10_rollback_sql(schema: str) -> str: """V10_ROLLBACK for a ledger schema, with the control schema's name QUOTED (a mixed-case or unusual name is a quoted identifier, as v10 created it).""" - quoted = '"' + control_schema_name(schema).replace('"', '""') + '"' - return V10_ROLLBACK.replace("__CONTROL__", quoted) + def quote(name: str) -> str: + return '"' + name.replace('"', '""') + '"' + + return V10_ROLLBACK.replace("__CONTROL__", quote(control_schema_name(schema))).replace("__LEDGER__", quote(validate_schema_name(schema))) MIGRATIONS: list[tuple[int, str]] = [(1, _V1), (2, _V2), (3, _V3), (4, _V4), (5, _V5), (6, _V6), (7, _V7), (8, _V8), (9, _V9), (10, _V10)] diff --git a/tests/test_tenant_keys.py b/tests/test_tenant_keys.py index 8a3dee1..8726970 100644 --- a/tests/test_tenant_keys.py +++ b/tests/test_tenant_keys.py @@ -525,3 +525,16 @@ def test_v10_and_its_rollback_work_for_a_mixed_case_ledger_schema(pg_server): with psycopg.connect(admin_dsn, autocommit=True) as conn: conn.execute(psycopg.sql.SQL("DROP SCHEMA IF EXISTS {} CASCADE").format(psycopg.sql.Identifier(schema))) conn.execute(psycopg.sql.SQL("DROP SCHEMA IF EXISTS {} CASCADE").format(psycopg.sql.Identifier(control_schema_name(schema)))) + + +@pg_only +def test_v10_rollback_sql_is_self_contained_and_ignores_the_callers_search_path(pg_schema): + """Codex round 3 on #110: schema_version must be qualified with the ledger schema, so a caller whose search_path points elsewhere still rolls back the right ledger (or fails), never another one.""" + assert migrate(pg_schema.admin_dsn, schema=pg_schema.schema, app_role="jarvis_app_test") == 10 + sql = v10_rollback_sql(pg_schema.schema) + assert f'"{pg_schema.schema}".schema_version' in sql + with psycopg.connect(pg_schema.admin_dsn, options="-c search_path=public") as conn: + conn.execute(sql) + with pg_schema.admin_conn() as conn: + assert conn.execute("SELECT max(version) FROM schema_version").fetchone()[0] == 9 + assert migrate(pg_schema.admin_dsn, schema=pg_schema.schema, app_role="jarvis_app_test") == 10