From 40840299624785ebb99271942c6e01b494044205 Mon Sep 17 00:00:00 2001 From: Matt Date: Tue, 2 Jun 2026 12:59:07 +0200 Subject: [PATCH] =?UTF-8?q?fix(audit):=20documenso=20=E2=80=94=20M2=20(res?= =?UTF-8?q?ervation=20EOI-milestone=20pollution),=20L11=20(v2=20numericId?= =?UTF-8?q?=20GET=20fallback),=20L12=20(API=20URL=20normalize/validate),?= =?UTF-8?q?=20L13=20(event=20dedup)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Opus 4.8 (1M context) --- src/lib/services/documenso-client.ts | 40 ++++++++++++-- src/lib/services/documents.service.ts | 79 +++++++++++++++++++-------- src/lib/services/port-config.ts | 15 ++++- src/lib/validators/settings.ts | 58 ++++++++++++++++++-- 4 files changed, 156 insertions(+), 36 deletions(-) diff --git a/src/lib/services/documenso-client.ts b/src/lib/services/documenso-client.ts index 29e617cb..fc624b06 100644 --- a/src/lib/services/documenso-client.ts +++ b/src/lib/services/documenso-client.ts @@ -459,6 +459,31 @@ export async function generateDocumentFromTemplate( portId, ).then(normalizeDocument); + // Resolve an authoritative numeric pk. `/template/use` returns the + // `envelope_xxx` string under `id`/`envelopeId` but does NOT reliably + // surface the internal numeric pk, so `created.numericId` is frequently + // null. DOCUMENT_COMPLETED (and other v2 webhooks) carry ONLY that + // numeric pk as `payload.id`, and `resolveWebhookDocument` matches it + // against `documents.documenso_numeric_id`. Persisting null there means + // the webhook resolves against neither column and the completion is + // dropped (signed PDF never downloads, stage never advances, no + // completion email/tenancy) until the poll worker reconciles by + // envelope id. Re-fetch the full envelope (GET /api/v2/envelope/{id}) + // when the numeric pk is missing so we persist a non-null value. + let numericId = created.numericId; + if (!numericId) { + try { + const fetched = await getDocument(created.id, portId); + if (fetched.numericId) numericId = fetched.numericId; + } catch (err) { + logger.warn( + { docId: created.id, err: err instanceof Error ? err.message : err }, + 'Documenso envelope re-fetch for numericId failed - documenso_numeric_id will be null; ' + + 'completion webhooks rely on the poll-worker reconciliation until then.', + ); + } + } + const desiredTitle = typeof v2Payload.title === 'string' && v2Payload.title.length > 0 ? v2Payload.title : null; // `/template/use` silently drops the `meta` field on the request body - @@ -571,11 +596,12 @@ export async function generateDocumentFromTemplate( const normalized = normalizeDocument({ envelopeId: distributed.id ?? created.id, // Distribute doesn't return the numeric id, so we synthesize it - // from the original /template/use response by passing the numeric - // id as Documenso's `id` field - normalizeDocument picks it up - // as numericId. Without this, the row would lose its numeric id - // on distribute and webhooks couldn't resolve back to it. - id: created.numericId, + // from the authoritative numericId resolved above (created response + // or envelope re-fetch) by passing it as Documenso's `id` field - + // normalizeDocument picks it up as numericId. Without this, the row + // would lose its numeric id on distribute and webhooks couldn't + // resolve back to it. + id: numericId, status: 'PENDING', recipients: distributed.recipients, }); @@ -585,7 +611,9 @@ export async function generateDocumentFromTemplate( { docId: created.id, err: err instanceof Error ? err.message : err }, 'Documenso envelope distribute failed - signingUrl will be null. Send-invitation will fail until the envelope is distributed.', ); - return created; + // Preserve the authoritative numericId resolved above so the persisted + // documenso_numeric_id is non-null even when distribute fails. + return { ...created, numericId }; } } diff --git a/src/lib/services/documents.service.ts b/src/lib/services/documents.service.ts index e5dcbf8d..07d5c492 100644 --- a/src/lib/services/documents.service.ts +++ b/src/lib/services/documents.service.ts @@ -845,34 +845,50 @@ export async function sendForSigning(documentId: string, portId: string, meta: A // Update interest if linked if (interest) { + // Always link the Documenso envelope to the interest, regardless of + // document type. The EOI-specific milestone columns + eoi_sent rule are + // gated below so a reservation_agreement send doesn't pollute EOI funnel + // data (dateEoiSent / eoiStatus / eoiDocStatus / eoi stage advance). await db .update(interests) .set({ documensoId: documensoDoc.id, - dateEoiSent: new Date(), - eoiStatus: 'waiting_for_signatures', updatedAt: new Date(), }) .where(eq(interests.id, interest.id)); - // Trigger berth rules - void evaluateRule('eoi_sent', interest.id, portId, meta); + // EOI documents only: stamp EOI milestones, fire the eoi_sent berth + // rule, and advance to the `eoi` stage. A reservation_agreement is not + // an EOI and must not touch these columns. + if (doc.documentType === 'eoi') { + await db + .update(interests) + .set({ + dateEoiSent: new Date(), + eoiStatus: 'waiting_for_signatures', + updatedAt: new Date(), + }) + .where(eq(interests.id, interest.id)); - // Advance pipeline stage to eoi (no-op if already further along). - // Doc sub-status is set by the webhook receiver when Documenso confirms; - // we stamp eoiDocStatus optimistically here so the UI shows "sent". - void advanceStageIfBehindGated( - interest.id, - portId, - 'eoi', - meta, - 'EOI sent for signing', - 'eoi_sent', - ); - await db - .update(interests) - .set({ eoiDocStatus: 'sent', updatedAt: new Date() }) - .where(eq(interests.id, interest.id)); + // Trigger berth rules + void evaluateRule('eoi_sent', interest.id, portId, meta); + + // Advance pipeline stage to eoi (no-op if already further along). + // Doc sub-status is set by the webhook receiver when Documenso confirms; + // we stamp eoiDocStatus optimistically here so the UI shows "sent". + void advanceStageIfBehindGated( + interest.id, + portId, + 'eoi', + meta, + 'EOI sent for signing', + 'eoi_sent', + ); + await db + .update(interests) + .set({ eoiDocStatus: 'sent', updatedAt: new Date() }) + .where(eq(interests.id, interest.id)); + } // Reservation agreements drive the reservation stage; the contract // pathway uses its own send call and stamps contractDocStatus. @@ -1752,11 +1768,21 @@ export async function handleDocumentCompleted(eventData: { documentId: string; p ); } - await db.insert(documentEvents).values({ - documentId: doc.id, - eventType: 'completed', - eventData: { documensoId: eventData.documentId }, - }); + // Idempotent insert: DOCUMENT_COMPLETED is retried by Documenso on + // receiver 5xx and re-reconciled by the poll worker. There is exactly one + // logical `completed` event per document, so use a deterministic + // per-document hash as the dedup key — the `idx_de_dedup` partial unique + // index on (documentId, signatureHash) + onConflictDoNothing collapses + // retries into a single timeline row instead of accumulating duplicates. + await db + .insert(documentEvents) + .values({ + documentId: doc.id, + eventType: 'completed', + signatureHash: `completed:${doc.id}`, + eventData: { documensoId: eventData.documentId }, + }) + .onConflictDoNothing(); emitToRoom(`port:${doc.portId}`, 'document:completed', { documentId: doc.id }); @@ -1915,6 +1941,11 @@ export async function handleDocumentOpened(eventData: { documentId: doc.id, eventType: 'viewed', signerId: signer?.id ?? null, + // recipient_email is the key for idx_de_per_recipient_dedup (partial + // unique on (documentId, recipientEmail, eventType) WHERE + // recipientEmail IS NOT NULL). Without it the index never engages and + // v2 multi-delivery RECIPIENT_VIEWED opens can't dedup. + recipientEmail: eventData.recipientEmail ?? null, signatureHash: eventData.signatureHash ?? null, eventData: { recipientEmail: eventData.recipientEmail }, }) diff --git a/src/lib/services/port-config.ts b/src/lib/services/port-config.ts index 041e340d..8b532ab2 100644 --- a/src/lib/services/port-config.ts +++ b/src/lib/services/port-config.ts @@ -10,6 +10,7 @@ import { env } from '@/lib/env'; import { normalizeBrandingUrl } from '@/lib/branding/url'; import { getSetting } from '@/lib/services/settings.service'; +import { normalizeDocumensoApiUrl } from '@/lib/validators/settings'; // ─── Setting key constants ─────────────────────────────────────────────────── @@ -430,18 +431,28 @@ export async function getPortDocumensoConfig(portId: string): Promise(SETTING_KEYS.publicSiteUrl, portId), ]); + // Defense-in-depth: normalize any persisted Documenso API URL override at + // read time. Writes are normalized + URL-validated by upsertSettingSchema, + // but values stored before that validation landed (or seeded directly) may + // still carry a `/api/v1`|`/api/v2` suffix that would double-path requests + // (documensoFetch appends `/api/vN/...` to this base). Strip it here too. + const normalizedApiUrl = + typeof apiUrl === 'string' && apiUrl.length > 0 + ? (normalizeDocumensoApiUrl(apiUrl) as string) + : apiUrl; + // Determine the resolution source for the two credentials. Used by // the documenso client to enrich 401/403 error messages so operators // can tell at a glance whether the failing key is per-port or env. type Source = 'port' | 'global' | 'env' | 'default' | 'none'; - const apiUrlSource: Source = apiUrl ? 'port' : env.DOCUMENSO_API_URL ? 'env' : 'none'; + const apiUrlSource: Source = normalizedApiUrl ? 'port' : env.DOCUMENSO_API_URL ? 'env' : 'none'; const apiKeySource: Source = apiKey ? 'port' : env.DOCUMENSO_API_KEY ? 'env' : 'none'; return { // Env values are now optional (admin is canonical). Empty / zero // defaults let consumers proceed and fail at the actual API call with // a clearer "not configured" error rather than crashing at type-check. - apiUrl: apiUrl ?? env.DOCUMENSO_API_URL ?? '', + apiUrl: normalizedApiUrl ?? env.DOCUMENSO_API_URL ?? '', apiKey: apiKey ?? env.DOCUMENSO_API_KEY ?? '', apiVersion: apiVersion ?? env.DOCUMENSO_API_VERSION, apiKeySource, diff --git a/src/lib/validators/settings.ts b/src/lib/validators/settings.ts index a7ebb88d..744f2ac2 100644 --- a/src/lib/validators/settings.ts +++ b/src/lib/validators/settings.ts @@ -1,9 +1,59 @@ import { z } from 'zod'; -export const upsertSettingSchema = z.object({ - key: z.string().min(1).max(100), - value: z.unknown(), -}); +/** Setting key holding the per-port Documenso API base URL override. */ +export const DOCUMENSO_API_URL_SETTING_KEY = 'documenso_api_url_override'; + +/** + * Normalize an admin-supplied Documenso API base URL. + * + * `documensoFetch` builds request URLs as `${baseUrl}/api/vN/...`, so the + * stored value must be a BARE host (no `/api/v1` or `/api/v2` suffix, no + * trailing slash). An admin pasting `https://sign.example.com/api/v1` would + * otherwise produce `https://sign.example.com/api/v1/api/v2/envelope/create` + * → 404 on every send/download, surfaced only as a generic + * DOCUMENSO_UPSTREAM_ERROR. Strip a trailing `/api/v1`|`/api/v2` and any + * trailing slashes so the value matches the `DOCUMENSO_API_URL` env-var + * contract (bare host only). Returns the input unchanged when it is not a + * non-empty string. + */ +export function normalizeDocumensoApiUrl(value: unknown): unknown { + if (typeof value !== 'string') return value; + const trimmed = value.trim(); + if (trimmed.length === 0) return trimmed; + // Strip a trailing `/api/v1` or `/api/v2` (with optional trailing slash), + // then strip any remaining trailing slashes. + return trimmed.replace(/\/api\/v[12]\/?$/i, '').replace(/\/+$/, ''); +} + +export const upsertSettingSchema = z + .object({ + key: z.string().min(1).max(100), + value: z.unknown(), + }) + .transform((input) => { + // Normalize + validate the Documenso API URL override on write. Other + // keys pass through untouched (value: z.unknown()). + if (input.key === DOCUMENSO_API_URL_SETTING_KEY && typeof input.value === 'string') { + return { ...input, value: normalizeDocumensoApiUrl(input.value) }; + } + return input; + }) + .superRefine((input, ctx) => { + if ( + input.key === DOCUMENSO_API_URL_SETTING_KEY && + typeof input.value === 'string' && + input.value.length > 0 + ) { + const result = z.string().url().safeParse(input.value); + if (!result.success) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + path: ['value'], + message: 'Documenso API URL must be a valid URL with no /api/v1 or /api/v2 suffix', + }); + } + } + }); export type UpsertSettingInput = z.infer;