diff --git a/src/lib/services/documents.service.ts b/src/lib/services/documents.service.ts index 6aa6074..2e1fef2 100644 --- a/src/lib/services/documents.service.ts +++ b/src/lib/services/documents.service.ts @@ -979,25 +979,50 @@ export async function handleDocumentCompleted(eventData: { documentId: string }) } } -export async function handleDocumentExpired(eventData: { documentId: string }) { - const doc = await db.query.documents.findFirst({ - where: eq(documents.documensoId, eventData.documentId), +export async function handleDocumentExpired(eventData: { documentId: string; portId?: string }) { + // Port-scoped lookup when the caller resolved a portId from the + // webhook signature. Two ports holding the same Documenso instance + // (or migrating between instances with id reuse) would otherwise + // share a documensoId across tenants, and findFirst would return + // whichever row sorted first — flipping a foreign-port document. + const matches = await db.query.documents.findMany({ + where: eventData.portId + ? and(eq(documents.documensoId, eventData.documentId), eq(documents.portId, eventData.portId)) + : eq(documents.documensoId, eventData.documentId), }); - if (!doc) { + + if (matches.length === 0) { logger.warn({ documensoId: eventData.documentId }, 'Document not found for webhook'); return; } + if (matches.length > 1 && !eventData.portId) { + // Cross-tenant ambiguity. Refuse to mutate without a resolved port — + // safer to drop the event (the cron expiry sweep will catch up) than + // flip the wrong document. + logger.error( + { + documensoId: eventData.documentId, + matchCount: matches.length, + ports: matches.map((m) => m.portId), + }, + 'Document expired webhook ambiguous across multiple ports — refusing to mutate', + ); + return; + } + + const doc = matches[0]!; + await db .update(documents) .set({ status: 'expired', updatedAt: new Date() }) - .where(eq(documents.id, doc.id)); + .where(and(eq(documents.id, doc.id), eq(documents.portId, doc.portId))); if (doc.interestId && doc.documentType === 'eoi') { await db .update(interests) .set({ eoiStatus: 'expired', updatedAt: new Date() }) - .where(eq(interests.id, doc.interestId)); + .where(and(eq(interests.id, doc.interestId), eq(interests.portId, doc.portId))); } await db.insert(documentEvents).values({ diff --git a/tests/integration/custom-fields.test.ts b/tests/integration/custom-fields.test.ts index fe41685..7ac7a27 100644 --- a/tests/integration/custom-fields.test.ts +++ b/tests/integration/custom-fields.test.ts @@ -315,3 +315,124 @@ describe('Custom Fields — Values', () => { expect(after.find((r) => r.definition.id === def.id)).toBeUndefined(); }); }); + +// ─── Cross-port Isolation ─────────────────────────────────────────────────── +// +// The previous suite seeded ONE port and verified CRUD inside it. The audit +// (HIGH §20 / auditor-J Issue 3) flagged that the suite never asserted that +// a definition created in port A is invisible from port B, nor that +// setValues refuses cross-port writes — combined with the deferred +// custom-fields-hardcoded-clients gap, no test would catch a regression. + +describe('Custom Fields — Cross-port Isolation', () => { + let portA: string; + let portB: string; + const userId = crypto.randomUUID(); + + beforeAll(async () => { + if (!dbAvailable) return; + portA = await seedPort(); + portB = await seedPort(); + }); + + afterAll(async () => { + if (!dbAvailable) return; + await cleanupPort(portA); + await cleanupPort(portB); + }); + + itDb('listDefinitions for port B does not see port A definitions', async () => { + const { createDefinition, listDefinitions } = + await import('@/lib/services/custom-fields.service'); + const metaA = makeAuditMeta({ portId: portA, userId }); + + await createDefinition( + portA, + userId, + { + entityType: 'client', + fieldName: 'port_a_only_field', + fieldLabel: 'Port A Only', + fieldType: 'text', + isRequired: false, + sortOrder: 0, + }, + metaA, + ); + + const portBDefs = await listDefinitions(portB, 'client'); + expect(portBDefs.find((d) => d.fieldName === 'port_a_only_field')).toBeUndefined(); + }); + + itDb('getValues from port B is empty for an entity scoped to port A', async () => { + const { createDefinition, setValues, getValues } = + await import('@/lib/services/custom-fields.service'); + const metaA = makeAuditMeta({ portId: portA, userId }); + const entityId = crypto.randomUUID(); + + const def = await createDefinition( + portA, + userId, + { + entityType: 'client', + fieldName: 'isolation_check', + fieldLabel: 'Isolation Check', + fieldType: 'text', + isRequired: false, + sortOrder: 0, + }, + metaA, + ); + await setValues(entityId, portA, userId, [{ fieldId: def.id, value: 'port-a-secret' }], metaA); + + const portBView = await getValues(entityId, portB); + expect(portBView.find((r) => r.definition.id === def.id)).toBeUndefined(); + }); + + itDb('setValues with a port-A fieldId from port B is rejected', async () => { + const { createDefinition, setValues } = await import('@/lib/services/custom-fields.service'); + const metaA = makeAuditMeta({ portId: portA, userId }); + const metaB = makeAuditMeta({ portId: portB, userId }); + const entityId = crypto.randomUUID(); + + const def = await createDefinition( + portA, + userId, + { + entityType: 'client', + fieldName: 'cross_port_write_check', + fieldLabel: 'Cross-port Write Check', + fieldType: 'text', + isRequired: false, + sortOrder: 0, + }, + metaA, + ); + + // Caller in port B tries to write a value keyed to port A's field id. + // The service must refuse — either by throwing, or by no-oping + // (returning without touching port A's data). Either way port A's + // value-store for the entity must remain unchanged. + let threw = false; + try { + await setValues(entityId, portB, userId, [{ fieldId: def.id, value: 'leak' }], metaB); + } catch { + threw = true; + } + + // Whether the service threw or silently dropped, no port-B value + // should now exist for this fieldId. We rely on the value lookup + // (rather than asserting the throw shape) so the test stays green + // across either remediation approach. + const { getValues } = await import('@/lib/services/custom-fields.service'); + const portBView = await getValues(entityId, portB); + const leaked = portBView.find((r) => r.definition.id === def.id); + if (!threw && leaked) { + // If the service silently no-oped AND returned the value: that's + // the failure mode the test is meant to catch. + expect(leaked).toBeUndefined(); + } else { + expect(leaked).toBeUndefined(); + } + }); +}); diff --git a/tests/integration/documenso-webhook-route.test.ts b/tests/integration/documenso-webhook-route.test.ts new file mode 100644 index 0000000..177b753 --- /dev/null +++ b/tests/integration/documenso-webhook-route.test.ts @@ -0,0 +1,123 @@ +/** + * Tests for the Documenso webhook receiver route at + * `src/app/api/webhooks/documenso/route.ts`. + * + * The receiver was previously only covered indirectly: the unit test + * `webhook-event-map.test.ts` validates the static event-name map, and + * `documents-expired-webhook.test.ts` calls handlers directly. Neither + * exercised the route's auth, dedup, dispatch loop, or 200-on-failure + * envelope. This file fills that gap. + * + * Refs: docs/audit-comprehensive-2026-05-05.md HIGH §19 (auditor-J Issue 2). + */ +import { describe, it, expect } from 'vitest'; +import { eq } from 'drizzle-orm'; +import { NextRequest } from 'next/server'; + +import { POST as documensoWebhook } from '@/app/api/webhooks/documenso/route'; +import { db } from '@/lib/db'; +import { documents, documentEvents } from '@/lib/db/schema/documents'; +import { env } from '@/lib/env'; +import { makeClient, makePort } from '../helpers/factories'; + +function buildRequest(body: Record, secret: string): NextRequest { + return new NextRequest('http://localhost:3000/api/webhooks/documenso', { + method: 'POST', + headers: { + 'content-type': 'application/json', + 'x-documenso-secret': secret, + }, + body: JSON.stringify(body), + }); +} + +describe('Documenso webhook route', () => { + it('returns 200 with ok:false when the secret header is missing or wrong', async () => { + const req = buildRequest( + { event: 'DOCUMENT_SIGNED', payload: { id: 'abc', recipients: [] } }, + 'wrong-secret', + ); + const res = await documensoWebhook(req); + expect(res.status).toBe(200); + const body = await res.json(); + expect(body).toMatchObject({ ok: false }); + }); + + it('with a valid secret + DOCUMENT_SIGNED writes a documentEvents row', async () => { + const port = await makePort(); + const client = await makeClient({ portId: port.id }); + + const documensoId = `docu-test-${Date.now()}`; + const [doc] = await db + .insert(documents) + .values({ + portId: port.id, + clientId: client.id, + documentType: 'eoi', + title: 'Webhook test EOI', + status: 'sent', + documensoId, + createdBy: 'seed', + }) + .returning(); + + const req = buildRequest( + { + event: 'DOCUMENT_SIGNED', + payload: { + id: documensoId, + recipients: [{ email: 'signer@test.invalid', signingStatus: 'SIGNED' }], + }, + }, + env.DOCUMENSO_WEBHOOK_SECRET, + ); + const res = await documensoWebhook(req); + expect(res.status).toBe(200); + + const events = await db + .select() + .from(documentEvents) + .where(eq(documentEvents.documentId, doc!.id)); + expect(events.length).toBeGreaterThanOrEqual(1); + }); + + it('replays of the same body are no-ops (signatureHash dedup)', async () => { + const port = await makePort(); + const client = await makeClient({ portId: port.id }); + + const documensoId = `docu-dedup-${Date.now()}`; + const [doc] = await db + .insert(documents) + .values({ + portId: port.id, + clientId: client.id, + documentType: 'eoi', + title: 'Dedup test EOI', + status: 'sent', + documensoId, + createdBy: 'seed', + }) + .returning(); + + const body = { + event: 'DOCUMENT_OPENED', + payload: { + id: documensoId, + recipients: [{ email: 'opener@test.invalid', readStatus: 'OPENED' }], + }, + }; + + await documensoWebhook(buildRequest(body, env.DOCUMENSO_WEBHOOK_SECRET)); + await documensoWebhook(buildRequest(body, env.DOCUMENSO_WEBHOOK_SECRET)); + + const events = await db + .select() + .from(documentEvents) + .where(eq(documentEvents.documentId, doc!.id)); + // The route's `handleDocumentOpened` writes an event with type + // `'viewed'`. One row from the first call; the second should have + // been refused by the signatureHash dedup guard. + const viewedEvents = events.filter((e) => e.eventType === 'viewed'); + expect(viewedEvents.length).toBe(1); + }); +}); diff --git a/tests/integration/documents-expired-webhook.test.ts b/tests/integration/documents-expired-webhook.test.ts index 6291239..5fa12ea 100644 --- a/tests/integration/documents-expired-webhook.test.ts +++ b/tests/integration/documents-expired-webhook.test.ts @@ -94,4 +94,57 @@ describe('handleDocumentExpired', () => { handleDocumentExpired({ documentId: 'definitely-not-a-real-doc' }), ).resolves.toBeUndefined(); }); + + it('does not flip a document in port B when port A receives the expired event', async () => { + // Two ports holding the same documenso_id (legacy data, or a future + // Documenso-instance migration that reuses ids). The handler currently + // mutates whichever document `findFirst` returns — locking in the + // intended behaviour now means a future port_id-aware handler can + // be added without regressing this guard. + // + // Per the audit, this assertion is expected to FAIL until the + // deferred port_id-on-handler fix lands. Once it lands, the test + // protects against re-introducing the cross-tenant flip. + const portA = await makePort(); + const portB = await makePort(); + const clientA = await makeClient({ portId: portA.id }); + const clientB = await makeClient({ portId: portB.id }); + + const sharedDocumensoId = `documenso-shared-${Date.now()}`; + const [docA] = await db + .insert(documents) + .values({ + portId: portA.id, + clientId: clientA.id, + documentType: 'eoi', + title: 'Port A EOI', + status: 'sent', + documensoId: sharedDocumensoId, + createdBy: 'seed', + }) + .returning(); + const [docB] = await db + .insert(documents) + .values({ + portId: portB.id, + clientId: clientB.id, + documentType: 'eoi', + title: 'Port B EOI', + status: 'sent', + documensoId: sharedDocumensoId, + createdBy: 'seed', + }) + .returning(); + + // Simulate port A receiving the expired event (the route caller would + // resolve this from the secret → port mapping in the deferred fix). + await handleDocumentExpired({ documentId: sharedDocumensoId, portId: portA.id }); + + // Port-A doc flipped, port-B unchanged. Pre-fix, both flip — this + // assertion locks the boundary in once the handler scope lands. + const afterA = await db.query.documents.findFirst({ where: eq(documents.id, docA!.id) }); + const afterB = await db.query.documents.findFirst({ where: eq(documents.id, docB!.id) }); + expect(afterA?.status).toBe('expired'); + expect(afterB?.status).toBe('sent'); + }); });