From 72bb0892222edc4012edf2c0835dde5547d2bd49 Mon Sep 17 00:00:00 2001 From: jonatanritter Date: Sat, 8 Aug 2026 15:32:20 -0300 Subject: [PATCH] fix(certificates): scope the live certificate uniquely per tenant, not just per branch_ref IMPORTANT (F2 review): certificate is per-tenant, matching series (already tenant-scoped) and the GET/DELETE anti-oracle boundary. Emission's _get_live_certificate and the upload-replace pre-check (certificates. service, renamed _get_live_certificate_by_branch -> _get_live_certificate_by_tenant_branch) both omitted tenant_ref -- two tenants of one product reusing branch_ref="matriz" collapsed onto the same slot: B's upload soft-deleted A's still-live certificate, and A's emission went on to sign with B's certificate. Migration 8f1a2c9d4b6e replaces the partial-unique index ix_fiscal_certificates_product_branch_live with ix_fiscal_certificates_product_tenant_branch_live on (product_id, tenant_ref, branch_ref) WHERE deleted_at IS NULL, with a working downgrade. Upload's two-layer defense (pre-check + IntegrityError -> CertificateUploadConflictError) still holds against the new index. Tests: - tests/emission/test_emissao.py:: test_dois_tenants_do_mesmo_produto_reusando_branch_ref_tem_certificados_isolados -- two tenants upload for the same product/branch_ref, both stay live; emission for each signs with its OWN certificate (observable via FIX 1's CNPJ check: without FIX 2, tenant A's emission would 409 emitente_certificate_cnpj_mismatch because the "live" cert would actually be B's). - tests/migrations/test_fiscal_documents_schema.py:: test_two_tenants_can_both_hold_a_live_certificate_for_the_same_branch_ref_on_real_migration -- real alembic upgrade head, raw INSERTs proving both tenants' certs land live. - tests/migrations/test_fiscal_documents_schema.py:: test_two_live_certificates_for_same_product_tenant_branch_violate_unique_index_on_real_migration (renamed from ..._product_branch_...) -- same (product, tenant, branch) still rejects a second live certificate on the real migration. - tests/certificates/test_certificates.py:: test_concurrent_uploads_for_same_product_branch_only_one_wins_the_other_gets_409 updated for the renamed/re-scoped precheck function (still same-tenant race, still 1 winner + 1 CertificateUploadConflictError). Co-Authored-By: Claude Opus 4.8 --- ...l_certificates_tenant_scoped_live_index.py | 53 ++++++++++++++ src/fiscal_svc/certificates/service.py | 60 +++++++++------- src/fiscal_svc/documents/models.py | 47 +++++++------ src/fiscal_svc/emission/service.py | 16 ++++- tests/certificates/test_certificates.py | 22 +++--- tests/emission/test_emissao.py | 65 +++++++++++++++++ .../test_fiscal_documents_schema.py | 69 +++++++++++++++++-- 7 files changed, 272 insertions(+), 60 deletions(-) create mode 100644 alembic/versions/8f1a2c9d4b6e_fiscal_certificates_tenant_scoped_live_index.py diff --git a/alembic/versions/8f1a2c9d4b6e_fiscal_certificates_tenant_scoped_live_index.py b/alembic/versions/8f1a2c9d4b6e_fiscal_certificates_tenant_scoped_live_index.py new file mode 100644 index 0000000..ebbad53 --- /dev/null +++ b/alembic/versions/8f1a2c9d4b6e_fiscal_certificates_tenant_scoped_live_index.py @@ -0,0 +1,53 @@ +"""fiscal_certificates: partial-unique live index scoped by tenant_ref too +(FIX 2, F2 review, 2026-07-17-sowai-fiscal-svc-design.md decisão #4) -- +`ix_fiscal_certificates_product_branch_live` (`(product_id, branch_ref) +WHERE deleted_at IS NULL`) let two DIFFERENT tenants of the SAME product +reusing an identical opaque `branch_ref` (e.g. both `"matriz"`) collapse +onto the SAME certificate slot: the second tenant's upload soft-deleted the +first tenant's still-live certificate as a legitimate "replace" instead of +a 409 conflict, and emission for the first tenant would go on to sign with +the second tenant's certificate. Replaces the index with one scoped +`(product_id, tenant_ref, branch_ref) WHERE deleted_at IS NULL` -- matching +`fiscal_series`'s own tenant-scoped uniqueness and the GET/DELETE +certificate lookups, which already filtered by `tenant_ref`. + +Revision ID: 8f1a2c9d4b6e +Revises: 30a80fe36910 +Create Date: 2026-07-24 00:00:00.000000 + +""" +from __future__ import annotations + +from typing import Sequence, Union + +import sqlalchemy as sa +from alembic import op + +revision: str = "8f1a2c9d4b6e" +down_revision: Union[str, Sequence[str], None] = "30a80fe36910" +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + + +def upgrade() -> None: + op.drop_index("ix_fiscal_certificates_product_branch_live", table_name="fiscal_certificates") + op.create_index( + "ix_fiscal_certificates_product_tenant_branch_live", + "fiscal_certificates", + ["product_id", "tenant_ref", "branch_ref"], + unique=True, + postgresql_where=sa.text("deleted_at IS NULL"), + ) + + +def downgrade() -> None: + op.drop_index( + "ix_fiscal_certificates_product_tenant_branch_live", table_name="fiscal_certificates" + ) + op.create_index( + "ix_fiscal_certificates_product_branch_live", + "fiscal_certificates", + ["product_id", "branch_ref"], + unique=True, + postgresql_where=sa.text("deleted_at IS NULL"), + ) diff --git a/src/fiscal_svc/certificates/service.py b/src/fiscal_svc/certificates/service.py index 057e738..fbe3642 100644 --- a/src/fiscal_svc/certificates/service.py +++ b/src/fiscal_svc/certificates/service.py @@ -73,12 +73,15 @@ class CertificateNotYetValidError(Exception): class CertificateUploadConflictError(Exception): """Traduz o `IntegrityError` da violação do índice parcial único - `ix_fiscal_certificates_product_branch_live` (`(product_id, branch_ref) - WHERE deleted_at IS NULL`, Task 3's `FiscalCertificate.__table_args__`) - -- disparado quando DUAS chamadas genuinamente concorrentes de - `upload_certificate` para o MESMO `(product_id, branch_ref)` ambas leem - `_get_live_certificate_by_branch() -> None` antes de qualquer uma - commitar. A PRIMEIRA a commitar vence; a segunda recebe este erro (409 + `ix_fiscal_certificates_product_tenant_branch_live` (`(product_id, + tenant_ref, branch_ref) WHERE deleted_at IS NULL`, Task 3's + `FiscalCertificate.__table_args__`, escopo estendido a `tenant_ref` na + correção do FIX 2/F2 review -- ver o docstring de `_get_live_ + certificate_by_tenant_branch`) -- disparado quando DUAS chamadas + genuinamente concorrentes de `upload_certificate` para o MESMO + `(product_id, tenant_ref, branch_ref)` ambas leem `_get_live_ + certificate_by_tenant_branch() -> None` antes de qualquer uma commitar. + A PRIMEIRA a commitar vence; a segunda recebe este erro (409 `certificate_upload_conflict` no router) em vez de silenciosamente criar um segundo certificado vivo.""" @@ -92,28 +95,35 @@ class CertificateUploadConflictError(Exception): def _is_fiscal_certificate_branch_live_violation(exc: IntegrityError) -> bool: - """True iff `exc` violates `ix_fiscal_certificates_product_branch_live` - -- substring match on the index name, which Task 3's migration names - explicitly (unlike the auto's un-named-in-create_all equivalent, this - one is identical across `Base.metadata.create_all` and Alembic, so a - plain substring check suffices, no "unique constraint" reinforcement + """True iff `exc` violates `ix_fiscal_certificates_product_tenant_branch_ + live` -- substring match on the index name, which Task 3's migration + names explicitly (unlike the auto's un-named-in-create_all equivalent, + this one is identical across `Base.metadata.create_all` and Alembic, so + a plain substring check suffices, no "unique constraint" reinforcement needed).""" detail = str(getattr(exc.orig, "args", [""])[0]) if exc.orig else str(exc) - return "ix_fiscal_certificates_product_branch_live" in detail + return "ix_fiscal_certificates_product_tenant_branch_live" in detail -async def _get_live_certificate_by_branch( - session: AsyncSession, product_id: uuid.UUID, branch_ref: str +async def _get_live_certificate_by_tenant_branch( + session: AsyncSession, product_id: uuid.UUID, tenant_ref: str, branch_ref: str ) -> FiscalCertificate | None: - """Scoped to `(product_id, branch_ref)` ONLY -- matches exactly the - scope of the partial unique index this function's callers (`upload_ - certificate`) need to pre-check against. NOT the public lookup for GET/ - DELETE (see `get_certificate`/`deactivate_certificate` below, which - additionally scope by `tenant_ref` for the anti-oracle boundary, design - spec decision #4: "anti-oracle 404 por (product_id, tenant_ref)").""" + """Scoped to `(product_id, tenant_ref, branch_ref)` -- matches exactly + the scope of the partial unique index this function's callers (`upload_ + certificate`) need to pre-check against. FIX 2 (F2 review): renamed + from `_get_live_certificate_by_branch` (which omitted `tenant_ref`) -- + without it, two DIFFERENT tenants of the SAME product reusing an + identical opaque `branch_ref` (e.g. both "matriz") collapsed onto the + SAME slot: the second tenant's upload soft-deleted the first tenant's + still-live certificate as a "replace", not a conflict. Same public + lookup shape as `_get_live_certificate` below (GET/DELETE, anti-oracle + boundary, design spec decision #4) -- kept as a SEPARATE function + because this one's caller (`upload_certificate`'s pre-check) needs + `None` on "no live cert for this slot", not a 404-worthy distinction.""" result = await session.execute( select(FiscalCertificate).where( FiscalCertificate.product_id == product_id, + FiscalCertificate.tenant_ref == tenant_ref, FiscalCertificate.branch_ref == branch_ref, FiscalCertificate.deleted_at.is_(None), ) @@ -159,9 +169,11 @@ async def upload_certificate( (`CertificateExpiredError`) e já vigente (`CertificateNotYetValidError`). Só depois de TODAS as checagens passarem é que qualquer escrita acontece: o certificado anterior (se houver, para o MESMO - `(product_id, branch_ref)`) é soft-deletado e o novo é inserido -- um - único VIVO por `(product_id, branch_ref)`, nunca dois, nunca um - update-in-place. + `(product_id, tenant_ref, branch_ref)`) é soft-deletado e o novo é + inserido -- um único VIVO por `(product_id, tenant_ref, branch_ref)`, + nunca dois, nunca um update-in-place (FIX 2/F2 review: `tenant_ref` + entrou no escopo -- ver `_get_live_certificate_by_tenant_branch`'s + docstring para o porquê). O INSERT final é protegido pelo índice parcial único (Task 3) contra a corrida de dois uploads genuinamente concorrentes -- o `commit()` do @@ -178,7 +190,7 @@ async def upload_certificate( if info.not_valid_before > now: raise CertificateNotYetValidError(info.not_valid_before) - previous = await _get_live_certificate_by_branch(session, product_id, branch_ref) + previous = await _get_live_certificate_by_tenant_branch(session, product_id, tenant_ref, branch_ref) if previous is not None: previous.deleted_at = now diff --git a/src/fiscal_svc/documents/models.py b/src/fiscal_svc/documents/models.py index bf48d32..73d09ec 100644 --- a/src/fiscal_svc/documents/models.py +++ b/src/fiscal_svc/documents/models.py @@ -109,27 +109,33 @@ class FiscalSeries(Base, UUIDPKMixin, TimestampMixin, SoftDeleteMixin): class FiscalCertificate(Base, UUIDPKMixin, TimestampMixin, SoftDeleteMixin): """Ported from the auto's `FiscalCertificate` (`app/modules/fiscal/ models.py`) -- A1 certificate (.pfx) for a `branch_ref`. Per the porte - table, the emitter is `(product_id, branch_ref)`, with `cnpj` carried - on the row itself (the "vínculo forte", design spec decision #4: the - CNPJ is what's validated against the certificate at upload and against - the emitente at emission -- `branch_ref` alone is an opaque string this - service never interprets). No fallback to a tenant-level or + table, the emitter is `(product_id, tenant_ref, branch_ref)`, with + `cnpj` carried on the row itself (the "vínculo forte", design spec + decision #4: the CNPJ is what's validated against the certificate at + upload and against the emitente at emission -- `branch_ref` alone is an + opaque string this service never interprets). No fallback to a product-level certificate -- fail-closed by construction, same as the - auto: a branch_ref without its own live certificate cannot emit. + auto: a `(tenant_ref, branch_ref)` without its own live certificate + cannot emit. - One VIVO (`deleted_at IS NULL`) row per `(product_id, branch_ref)`: a - second upload soft-deletes the previous live row (Task 4's - `certificates.service`). `ix_fiscal_certificates_product_branch_live` - is a PARTIAL UNIQUE index enforcing that at the database level (same - "two concurrent uploads must not both land a live row" reasoning as the - auto's own migration `a1b2c3d4e5f6`'s fix) -- scoped by `product_id` in - ADDITION to `branch_ref` (the auto's version only needed `branch_id`, - already product-scoped by being a real FK; here `branch_ref` is an - OPAQUE string owned by the calling product, so two DIFFERENT products - could coincidentally pick the identical string for two DIFFERENT real - branches -- scoping the uniqueness by `product_id` too is what keeps - that from cross-contaminating one product's certificate slot with - another's). + One VIVO (`deleted_at IS NULL`) row per `(product_id, tenant_ref, + branch_ref)`: a second upload soft-deletes the previous live row + (Task 4's `certificates.service`). `ix_fiscal_certificates_product_ + tenant_branch_live` is a PARTIAL UNIQUE index enforcing that at the + database level (same "two concurrent uploads must not both land a live + row" reasoning as the auto's own migration `a1b2c3d4e5f6`'s fix) -- + scoped by `product_id` AND `tenant_ref` in ADDITION to `branch_ref` + (the auto's version only needed `branch_id`, already product-scoped by + being a real FK; here `branch_ref` is an OPAQUE string owned by the + calling product, so two DIFFERENT products -- or two DIFFERENT tenants + of the SAME product -- could coincidentally pick the identical string + for two DIFFERENT real branches -- scoping the uniqueness by + `product_id`+`tenant_ref` too is what keeps that from + cross-contaminating one tenant's certificate slot with another's; FIX 2, + F2 review, 2026-07-17-sowai-fiscal-svc-design.md decisão #4 turned this + into a shipped bug otherwise: two tenants both using `branch_ref= + "matriz"` would collapse onto one slot, the second tenant's upload + silently soft-deleting the first's still-live certificate). `pfx_encrypted`/`password_encrypted` are Fernet ciphertext (Task 4, `FISCAL_CERT_ENCRYPTION_KEY` env var, own key -- never @@ -139,8 +145,9 @@ class FiscalCertificate(Base, UUIDPKMixin, TimestampMixin, SoftDeleteMixin): __tablename__ = "fiscal_certificates" __table_args__ = ( Index( - "ix_fiscal_certificates_product_branch_live", + "ix_fiscal_certificates_product_tenant_branch_live", "product_id", + "tenant_ref", "branch_ref", unique=True, postgresql_where=text("deleted_at IS NULL"), diff --git a/src/fiscal_svc/emission/service.py b/src/fiscal_svc/emission/service.py index d139ddf..c4d7e0a 100644 --- a/src/fiscal_svc/emission/service.py +++ b/src/fiscal_svc/emission/service.py @@ -239,11 +239,23 @@ def _normalize_cnpj(value: str) -> str: async def _get_live_certificate( - session: AsyncSession, product_id: uuid.UUID, branch_ref: str + session: AsyncSession, product_id: uuid.UUID, tenant_ref: str, branch_ref: str ) -> FiscalCertificate | None: + """Escopado por `(product_id, tenant_ref, branch_ref)` -- FIX 2 (F2 + review): o certificado é POR TENANT (mesma escala que `_get_series`, já + tenant-scoped, e o GET/DELETE de `certificates.service._get_live_ + certificate`). Antes desta correção este lookup omitia `tenant_ref`, e + dois tenants do MESMO produto reusando um `branch_ref` idêntico (ex.: + ambos "matriz", nomes opacos que o serviço nunca interpreta) resolviam + para o MESMO certificado -- o segundo upload de um tenant B para + "matriz" soft-deletava o certificado vivo do tenant A (ver o índice + parcial único em `documents.models.FiscalCertificate`, também corrigido + nesta mesma revisão) e a emissão de A passava a assinar com o + certificado de B.""" result = await session.execute( select(FiscalCertificate).where( FiscalCertificate.product_id == product_id, + FiscalCertificate.tenant_ref == tenant_ref, FiscalCertificate.branch_ref == branch_ref, FiscalCertificate.deleted_at.is_(None), ) @@ -303,7 +315,7 @@ async def emitir_documento( # 2. completude estrutural -- ANTES de alocar número. missing: list[str] = [] - certificate = await _get_live_certificate(session, product.id, payload.branch_ref) + certificate = await _get_live_certificate(session, product.id, payload.tenant_ref, payload.branch_ref) if certificate is None: missing.append("certificado A1 do branch_ref") elif certificate.not_valid_after < datetime.now(timezone.utc): diff --git a/tests/certificates/test_certificates.py b/tests/certificates/test_certificates.py index de7505e..f93699e 100644 --- a/tests/certificates/test_certificates.py +++ b/tests/certificates/test_certificates.py @@ -355,11 +355,11 @@ async def test_concurrent_uploads_for_same_product_branch_only_one_wins_the_othe Sem sincronização explícita, as duas corrotinas rodam no MESMO event loop e podem interleavear de um jeito que NÃO exercita a corrida real: se a primeira `upload_certificate` COMMITA inteiro antes de a segunda - fazer o pre-check `_get_live_certificate_by_branch`, a segunda enxerga a - linha viva da primeira e faz um REPLACE LEGÍTIMO (soft-delete + insert) - -- 2 sucessos, 1 linha viva, comportamento CORRETO do serviço, mas que - quebraria a asserção abaixo (que exige exatamente 1 sucesso + 1 - conflito). Mesma técnica de sincronização determinística de + fazer o pre-check `_get_live_certificate_by_tenant_branch`, a segunda + enxerga a linha viva da primeira e faz um REPLACE LEGÍTIMO (soft-delete + + insert) -- 2 sucessos, 1 linha viva, comportamento CORRETO do + serviço, mas que quebraria a asserção abaixo (que exige exatamente 1 + sucesso + 1 conflito). Mesma técnica de sincronização determinística de `auto/backend/tests/modules/financeiro/test_pay_account_payable.py:: test_pay_concurrent_with_cancel_via_http_lock_serializes_the_race`: monkeypatch no ponto de await entre o pre-check e o commit, com um @@ -373,15 +373,15 @@ async def test_concurrent_uploads_for_same_product_branch_only_one_wins_the_othe pfx_a = _build_test_pfx(cnpj="14200166000187", password="senha123", cn="A:14200166000187") pfx_b = _build_test_pfx(cnpj="14200166000187", password="senha456", cn="B:14200166000187") - original_precheck = certificate_service._get_live_certificate_by_branch + original_precheck = certificate_service._get_live_certificate_by_tenant_branch precheck_done = asyncio.Event() first_precheck_claimed = False - async def _precheck_forcing_both_before_any_commit(session, product_id, branch_ref): + async def _precheck_forcing_both_before_any_commit(session, product_id, tenant_ref, branch_ref): nonlocal first_precheck_claimed if not first_precheck_claimed: first_precheck_claimed = True - result = await original_precheck(session, product_id, branch_ref) + result = await original_precheck(session, product_id, tenant_ref, branch_ref) precheck_done.set() # Segura ESTA chamada (ainda antes do commit em upload_certificate) # até depois que a outra também tenha feito seu pre-check -- @@ -390,10 +390,12 @@ async def test_concurrent_uploads_for_same_product_branch_only_one_wins_the_othe await asyncio.sleep(0.3) return result await precheck_done.wait() - return await original_precheck(session, product_id, branch_ref) + return await original_precheck(session, product_id, tenant_ref, branch_ref) monkeypatch.setattr( - certificate_service, "_get_live_certificate_by_branch", _precheck_forcing_both_before_any_commit + certificate_service, + "_get_live_certificate_by_tenant_branch", + _precheck_forcing_both_before_any_commit, ) session_maker = async_sessionmaker(test_engine, expire_on_commit=False) diff --git a/tests/emission/test_emissao.py b/tests/emission/test_emissao.py index e15d4d6..b76ec64 100644 --- a/tests/emission/test_emissao.py +++ b/tests/emission/test_emissao.py @@ -410,6 +410,71 @@ async def test_emitente_cnpj_divergente_do_certificado_e_409_e_nao_queima_numero assert success_response.json()["numero"] == raw["numero"] +# --- FIX 2 (F2 review): certificado é POR TENANT, não só por branch_ref ----- + + +@pytest.mark.asyncio +async def test_dois_tenants_do_mesmo_produto_reusando_branch_ref_tem_certificados_isolados(db_session): + """Antes do FIX 2, `_get_live_certificate` (emissão) e o pre-check de + upload omitiam `tenant_ref` -- dois tenants do MESMO produto reusando o + MESMO `branch_ref` opaco ("matriz", plausível: refs são strings livres + do produto chamador) colapsavam no MESMO slot. O upload do tenant B + soft-deletava o certificado ainda vivo do tenant A (replace + "legítimo"), e a emissão do tenant A passava a resolver o certificado + de B. + + A prova combina FIX 1 (CNPJ do emitente vs certificado) para tornar o + vínculo OBSERVÁVEL: sem o FIX 2, o certificado "vivo" para `branch_ref + ="matriz"` seria o de B (CNPJ_B) para AMBOS os tenants -- a emissão do + tenant A com `emitente.cnpj`=CNPJ_A bateria no FIX 1 e devolveria 409 + `emitente_certificate_cnpj_mismatch` em vez de 201.""" + cnpj_a = "11222333000181" + cnpj_b = "44555666000107" + product, key = await _product_and_key(db_session) + payload_a, raw_a = _payload_from_golden( + "caso_padrao_intra", tenant_ref="tenant-a", branch_ref="matriz" + ) + payload_a["emitente"] = {**payload_a["emitente"], "cnpj": cnpj_a} + payload_b, raw_b = _payload_from_golden( + "caso_padrao_inter", tenant_ref="tenant-b", branch_ref="matriz" + ) + payload_b["emitente"] = {**payload_b["emitente"], "cnpj": cnpj_b} + + transport = ASGITransport(app=app) + async with AsyncClient(transport=transport, base_url="http://test") as client: + await _setup_certificate_and_series( + db_session, client, key, tenant_ref="tenant-a", branch_ref="matriz", + serie=raw_a["serie"], next_number=raw_a["numero"], cnpj=cnpj_a, + ) + # Segundo upload, MESMO produto, MESMO branch_ref, tenant DIFERENTE. + await _setup_certificate_and_series( + db_session, client, key, tenant_ref="tenant-b", branch_ref="matriz", + serie=raw_b["serie"], next_number=raw_b["numero"], cnpj=cnpj_b, + ) + + # O certificado de A segue vivo (GET de A não foi soft-deletado + # pelo upload de B) -- prova direta, sem depender do FIX 1. + get_a = await client.get( + "/v1/certificados", + params={"tenant_ref": "tenant-a", "branch_ref": "matriz"}, + headers=_headers(key), + ) + assert get_a.status_code == 200, get_a.text + assert get_a.json()["cnpj_certificado"] == cnpj_a + + emit_a = await client.post( + "/v1/emissoes", json=payload_a, + headers={**_headers(key), "Idempotency-Key": f"idem-a-{uuid.uuid4().hex}"}, + ) + emit_b = await client.post( + "/v1/emissoes", json=payload_b, + headers={**_headers(key), "Idempotency-Key": f"idem-b-{uuid.uuid4().hex}"}, + ) + + assert emit_a.status_code == 201, emit_a.text + assert emit_b.status_code == 201, emit_b.text + + # --- prova do outbox -------------------------------------------------------- diff --git a/tests/migrations/test_fiscal_documents_schema.py b/tests/migrations/test_fiscal_documents_schema.py index ca99a23..d797fc8 100644 --- a/tests/migrations/test_fiscal_documents_schema.py +++ b/tests/migrations/test_fiscal_documents_schema.py @@ -231,13 +231,16 @@ async def test_chave_acesso_unique_constraint_holds_on_real_migration(migration_ @pytest.mark.asyncio -async def test_two_live_certificates_for_same_product_branch_violate_unique_index_on_real_migration( +async def test_two_live_certificates_for_same_product_tenant_branch_violate_unique_index_on_real_migration( migration_database, ): """Same fix/reasoning as the auto's `a1b2c3d4e5f6` migration: a PARTIAL - UNIQUE index (here on `(product_id, branch_ref) WHERE deleted_at IS - NULL`) makes two concurrently-uploaded LIVE certificates for the same - slot structurally impossible, not just avoided by the service layer.""" + UNIQUE index (here on `(product_id, tenant_ref, branch_ref) WHERE + deleted_at IS NULL` -- FIX 2/F2 review, migration `8f1a2c9d4b6e`) makes + two concurrently-uploaded LIVE certificates for the SAME + `(product_id, tenant_ref, branch_ref)` slot structurally impossible, not + just avoided by the service layer. Renamed from `..._product_branch_...` + (pre-FIX-2 name) -- `tenant_ref` is now PART of the scope this proves.""" result = run_alembic(_MIGRATION_DB_URL, "upgrade", "head") assert result.returncode == 0, result.stderr @@ -274,6 +277,64 @@ async def test_two_live_certificates_for_same_product_branch_violate_unique_inde await engine.dispose() +@pytest.mark.asyncio +async def test_two_tenants_can_both_hold_a_live_certificate_for_the_same_branch_ref_on_real_migration( + migration_database, +): + """FIX 2 (F2 review): the pre-fix index was `(product_id, branch_ref) + WHERE deleted_at IS NULL` -- ONE live cert per `branch_ref` PER PRODUCT, + regardless of tenant. Two tenants of the SAME product reusing the + identical opaque `branch_ref="matriz"` collided on that slot: this + proves, on a database built PURELY by `alembic upgrade head`, that BOTH + now insert and stay live simultaneously -- the index scope is + `(product_id, tenant_ref, branch_ref)`.""" + result = run_alembic(_MIGRATION_DB_URL, "upgrade", "head") + assert result.returncode == 0, result.stderr + + from fiscal_svc.documents.models import FiscalCertificate + + engine = create_async_engine(_MIGRATION_DB_URL, echo=False) + session_maker = async_sessionmaker(engine, expire_on_commit=False) + try: + async with session_maker() as session: + product = await _make_product(session, name="auto-cert-multi-tenant") + + def _cert(tenant_ref, cnpj): + return FiscalCertificate( + product_id=product.id, + tenant_ref=tenant_ref, + branch_ref="matriz", + cnpj=cnpj, + pfx_encrypted=b"\x00pfx", + password_encrypted=b"\x00pw", + subject_cn=f"EMPRESA TESTE LTDA:{cnpj}", + cnpj_certificado=cnpj, + not_valid_before=datetime.now(timezone.utc) - timedelta(days=1), + not_valid_after=datetime.now(timezone.utc) + timedelta(days=365), + ) + + cert_a = _cert("tenant-a", "14200166000280") + cert_b = _cert("tenant-b", "99887766000155") + session.add(cert_a) + session.add(cert_b) + # Would raise IntegrityError on the old (product_id, branch_ref) + # index before FIX 2 -- the second INSERT collided with the + # first tenant's still-live row. + await session.commit() + + cert_a_id, cert_b_id = cert_a.id, cert_b.id + + async with session_maker() as session: + reloaded_a = await session.get(FiscalCertificate, cert_a_id) + reloaded_b = await session.get(FiscalCertificate, cert_b_id) + assert reloaded_a.deleted_at is None + assert reloaded_b.deleted_at is None + assert reloaded_a.cnpj_certificado == "14200166000280" + assert reloaded_b.cnpj_certificado == "99887766000155" + finally: + await engine.dispose() + + @pytest.mark.asyncio async def test_duplicate_fiscal_series_tuple_violates_unique_constraint_on_real_migration( migration_database,