fix(audit): documenso — M2 (reservation EOI-milestone pollution), L11 (v2 numericId GET fallback), L12 (API URL normalize/validate), L13 (event dedup)
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -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 };
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -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 },
|
||||
})
|
||||
|
||||
@@ -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<PortDocume
|
||||
readSetting<string>(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,
|
||||
|
||||
@@ -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<typeof upsertSettingSchema>;
|
||||
|
||||
|
||||
Reference in New Issue
Block a user