b10bf9bf8eb99e23e627f8f9ede4fd2dc63b1e79
9 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
| ecf49be18c |
fix(audit-wave-10): concurrency hardening (concurrency-auditor)
Close the CRITICAL + HIGH-tractable race conditions the concurrency-auditor flagged. The wide-impact items (BullMQ jobId plumbing — C-2; webhook outbound retry idempotency keys; etc.) span too many call sites for a single contained wave and stay deferred. **C-1 — handleDocumentCompleted concurrent-retry orphan-blob** Wave 1 fixed the compensating-delete on single-process failure but the idempotency gate at line 1110 reads `doc.status` outside any row lock. Two webhook deliveries arriving in parallel both pass the gate, both storage.put + db.insert(files), and the losing files row orphans its blob since documents.signed_file_id only points at one. Now the transaction at line 1176 SELECTs the document `FOR UPDATE` and re-checks the gate; if a concurrent worker already completed, throws a sentinel `DocumentAlreadyCompletedError` which the outer catch recognizes and runs the compensating storage.delete at info level (not error). Net effect: at-most-once signed-PDF persistence even under Documenso 5xx-then-retry storms. **H-1 — moveFolder cycle check race** Two concurrent folder moves (A → B and B → A) in READ COMMITTED can each pass the cycle check against pre-state and both commit, leaving A↔B in the tree. Add a per-port `pg_advisory_xact_lock` at the top of the move transaction so the walk-and-write is atomic per port. Lock auto-releases on tx end; no impact on cross-port folder ops. **H-3 — upsertInterestBerth 23505 → generic 500** Two concurrent `setPrimaryBerth` calls hit `idx_interest_berths_one_primary` and the loser surfaced as a generic 500. Catch the 23505 + constraint name and remap to ConflictError so the UI gets a "Another rep changed the primary berth at the same time. Refresh and try again." toast. **M-2 — username uniqueness 23505 → generic 500** Same TOCTOU shape: pre-check at me/route.ts:132 says "available", the UPDATE then fails at the partial unique index. Catch 23505 + `idx_user_profiles_username_unique` and remap to ConflictError. Tests 1315/1315. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
|||
| c0e5af8b92 |
fix(sales): wire missing berth-rule triggers + portal company-billed invoices
- G-C4: deposit_received in invoices.ts - G-C4 + G-I2: interest_archived + notifyNextInLine in archiveInterest - G-C4: interest_completed in setInterestOutcome - G-C4: berth_unlinked in removeInterestBerth - G-I5: portal invoices include billingEntityType='company' when client is the director Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
|||
Matt Ciaccio
|
4eea4ceff9 |
fix(audit-tier-4): tenant-isolation defense-in-depth
Closes the audit's HIGH §10 + MED §§17–22 isolation footguns. None of these are user-impactful TODAY — every site is preceded by a port- scoped read or pre-validated by ctx.portId — but each is a future- refactor accident waiting to happen, so the SQL itself now pins the tenant boundary: * mergeClients gains a callerPortId option; the route caller passes ctx.portId. removeInterestBerth now requires portId and verifies both the interest and the berth share it before deleting the junction row. All three callers updated. * Six service mutations now scope the WHERE to (id, portId): form-templates update + delete, invoices.detectOverdue per-row update, notifications.markRead, clients.deleteRelationship. company-memberships uses an inArray sub-select against port companies (no port_id column on the table itself), covering updateMembership / endMembership / setPrimary. * Port-scoped file lookups in portal.getDocumentDownloadUrl, reports.getDownloadUrl (file presign), berth-reservations.activate (contractFileId attach guard), and residential.getResidentialInterestById (residentialClient join). Test status: 1168/1168 vitest, tsc clean. Refs: docs/audit-comprehensive-2026-05-05.md HIGH §10 + MED §§17–22 (auditor-B3 Issues 1–5,7). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
Matt Ciaccio
|
4723994bdc |
feat(errors): platform-wide request ids + error codes + admin inspector
End-to-end error-handling overhaul. A user hitting any failure now sees
a plain-text message + stable error code + reference id. A super admin
can paste the id into /admin/errors/<id> for the full request shape,
sanitized body, error stack, and a heuristic likely-cause hint.
REQUEST CONTEXT (AsyncLocalStorage)
- src/lib/request-context.ts mints a per-request frame carrying
requestId + portId + userId + method + path + start timestamp.
- withAuth wraps every authenticated handler in runWithRequestContext
and accepts an upstream X-Request-Id header (validated shape) or
generates a fresh UUID. The id ALWAYS leaves on the X-Request-Id
response header, including early-return 401/403/4xx paths.
- Pino logger reads from the same context via mixin — every log
line emitted during the request automatically carries the ids
with no per-call threading.
ERROR CODE REGISTRY
- src/lib/error-codes.ts defines stable DOMAIN_REASON codes with
HTTP status + plain-text user-facing message (no jargon, written
for the rep on the phone with a customer).
- New CodedError class wraps a registered code + optional
internalMessage (admin-only — never sent to client).
- Existing AppError subclasses got plain-text default rewrites so
legacy throw sites improve immediately without migration.
- High-impact services migrated to specific codes:
expenses (RECEIPT_REQUIRED, INVOICE_LINKED), interest-berths
(CROSS_PORT_LINK_REJECTED), berth-pdf (PDF_MAGIC_BYTE / PDF_EMPTY /
PDF_TOO_LARGE / VERSION_ALREADY_CURRENT), recommender
(INTEREST_PORT_MISMATCH).
ERROR ENVELOPE
- errorResponse always sets X-Request-Id header + requestId field.
- 5xx responses include a "Quote error ID …" friendly line.
- 4xx kept clean (validation, permission, not-found don't pollute
the inspector — they're already in audit log).
PERSISTENCE (error_events table, migration 0040)
- One row per 5xx, keyed on requestId, with method/path/status/error
name+message/stack head (4KB cap)/sanitized body excerpt (1KB cap;
password/token/secret/etc keys redacted)/duration/IP/UA/metadata.
- captureErrorEvent extracts Postgres SQLSTATE/severity/cause.code
so the classifier can recognize FK / unique / NOT NULL / schema-
drift violations.
- Failure to persist is logged-not-thrown.
LIKELY-CULPRIT CLASSIFIER (src/lib/error-classifier.ts)
- 4-pass heuristic (first match wins):
1. Postgres SQLSTATE → human reason (23503 FK, 23505 unique,
42703 schema drift, 53300 connection limit, …)
2. Error class name (AbortError, TimeoutError, FetchError,
ZodError)
3. Stack-path patterns (/lib/storage/, /lib/email/, documenso,
openai|claude, /queue/workers/)
4. Free-text message keywords (econnrefused, rate limit, timeout,
unauthorized|invalid api key)
- Returns { label, hint, subsystem } for the inspector badge.
CLIENT SIDE
- apiFetch throws structured ApiError with message + code + requestId
+ details + retryAfter.
- toastError() helper renders the standard 3-line toast:
plain message / Error code: X / Reference ID: Y [Copy ID].
ADMIN INSPECTOR
- /<port>/admin/errors lists captured 5xx with status badge + path +
likely-culprit badge + truncated message + reference id. Filter by
status code; auto-refresh via TanStack Query.
- /<port>/admin/errors/<requestId> deep-dive: request shape, full
error name+message+stack, sanitized body excerpt, raw metadata,
registered-code lookup (so admin can compare to what user saw),
likely-culprit hint with subsystem tag.
- /<port>/admin/errors/codes is the in-app code reference page —
every registered code grouped by domain prefix, searchable, with
HTTP status + user message inline. Linked from inspector header
so admins can flip to it while triaging.
- Permission: admin.view_audit_log. Super admins see all ports;
regular admins port-scoped.
- system-monitoring dashboard now surfaces error_events alongside
permission_denied audit + queue failed jobs (RecentError gains
source: 'request' variant).
DOCS
- docs/error-handling.md walks through coded errors, plain-text
message guidelines, client toasting, admin inspector usage,
persistence rules, classifier internals, pruning, and the
legacy → CodedError migration path.
MIGRATION SAFETY
- Audit confirmed all 41 migrations (0000-0040) apply cleanly in
journal order against an empty DB. 0040 references ports(id)
which exists from 0000. 0035/0038 don't deadlock under sequential
psql -f. Removed redundant idx_ds_sent_by from 0038 (created in
0037).
Tests: 1168/1168 vitest passing. tsc clean.
- security-error-responses tests updated for plain-text messages
+ new optional response keys (code/requestId/message).
- berth-pdf-versions tests assert stable error codes via
toMatchObject({ code }) rather than message regex.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
||
|
Matt Ciaccio
|
180912ba9f |
fix(audit-final): pre-merge hardening + expense receipt UI
Final audit pass on feat/berth-recommender (3 parallel Opus agents) caught 5 critical and ~12 high-severity findings. All addressed in-branch; medium/low items deferred to docs/audit-final-deferred.md. Critical: - Add filesystem-backend PUT handler at /api/storage/[token] so presigned uploads stop 405-ing in filesystem mode (every browser-driven berth-PDF + brochure upload was broken). Same token-verify + replay protection as GET, plus magic-byte gate when c=application/pdf. - Forward req.signal into streamExpensePdf so an aborted 1000-receipt export no longer keeps grinding for minutes. - Strengthen Content-Disposition filename sanitization: \s matches CR/LF which would let documentName forge headers; restrict to [\w. -]+ and add filename* RFC 5987 fallback. - Lock public berths feed behind an explicit slug allowlist instead of ?portSlug= enumeration. - Reject cross-port interest_berths upserts (defense-in-depth on top of the recommender SQL port filter). High: - Recommender: width-only feasibility now caps length via L/W ratio so a 200ft berth doesn't surface for a 30ft beam request; total_interest_count filters out junction rows whose interest is in another port. - Mooring normalization follow-up migration (0034) catches un-hyphenated padded forms (A01) the original 0024 WHERE missed. - Send-out rate limit moved AFTER validation and scoped per-(port, user) so typos don't burn a slot and a multi-port rep can't be DoS'd by another tenant. - Default-brochure path now blocks an archived row from sneaking through the partial unique index. - NocoDB import --update-snapshot honoured under --dry-run so reps can refresh the seed JSON without committing DB writes. - PDF export: orderBy desc(expenseDate); apply isNull(archivedAt) when expenseIds are passed (was bypassed); flag rate-unavailable rows with an amber footer instead of silently treating them as 1:1; skip the USD->EUR chain when source already matches target. - expense-form-dialog: revokeObjectURL captures the URL in the closure instead of revoking the still-displayed one; reset upload state on close. - scan/page: handleClearReceipt resets in-flight scan/upload mutations; Save disabled while upload pending. - updateExpense re-asserts receipt-or-acknowledgement at the merged row so PATCH can't slip past the create-time refine. Plus the in-progress receipt upload UI for the expense form dialog (receipt picker + "I have no receipt" checkbox + warning banner) and a noReceiptAcknowledged flag on ExpenseRow for edit-mode hydration. Includes the canonical plan doc (referenced in CLAUDE.md), the handoff prompt, and a deferred-findings index for follow-up issues. 1163/1163 vitest passing. Typecheck clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
Matt Ciaccio
|
86372a857f |
fix(audit): post-review hardening across phases 0-7
15 of 17 findings from the consolidated audit (3 reviewer agents on the previously-shipped phase commits). Remaining two are nice-to-have follow-ups deferred. Critical (data integrity / security): - Public berths API: closed-deal junction rows no longer flip a berth to "Under Offer" - filter on `interests.outcome IS NULL` so won/ lost/cancelled don't pollute public-map status. Both list + single-mooring routes. - Recommender heat: cancelled outcomes now count as fall-throughs (SQL was `LIKE 'lost%'` which silently dropped them, leaving cancelled-only berths stuck in tier A). - Filesystem presignDownload returns an absolute URL (origin from APP_URL) so emailed download links resolve from external mail clients. - Magic-byte verification on the presigned-PUT path: both per-berth PDFs and brochures stream the first 5 bytes via the storage backend and reject + delete on `%PDF-` mismatch (was only enforced when the server saw the buffer; presign-PUT was wide open). - Replay-protection TTL aligned to the token's own expiry (was a fixed 30 min, but send-out tokens live 24 h). Floor 60 s, ceiling 25 days. - Brochures unique partial index on (port_id) WHERE is_default=true + 0032 migration. Closes the read-then-write race in the create/ update transactions. Important: - Recommender SQL: defense-in-depth `i.port_id = $portId` filter on the aggregates CTE. - berth-pdf service: per-berth pg_advisory_xact_lock around the version-number SELECT + insert. Storage key is now UUID-based so concurrent uploads can't collide on blob paths. Replaces `nextVersionNumber` with the tx-bound variant. - berth-pdf apply: rejects with ConflictError when parse_results contain a mooring-mismatch warning unless the caller passes `confirmMooringMismatch: true` (force-reconfirm gate was UI-only). - Send-out body: HTML-escape brochure filename in the download-link fallback (XSS guard). - parseDecimalWithUnit rejects negative numbers. - listClients DISTINCT ON for primary contact resolution: bounds contact-row count to ~2 per client. Defensive: - verifyProxyToken rejects NaN/Infinity expiries via Number.isFinite. - Replaced sql ANY() with inArray() in interest-berths. Tests: 1145 -> 1163 passing. Deferred: bulk-send rate limit (no bulk endpoint today), markdown italic regex breaking links with asterisks (cosmetic). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
Matt Ciaccio
|
b4776b4c3c |
feat(interests): linked berths list with role-flag toggles + EOI bypass
Implements plan §5.5: a per-interest "Linked berths" panel mounted above the
recommender on the interest detail Overview tab. Each junction row exposes
the role-flag controls reps need to manage the M:M `interest_berths` link
without the legacy single-berth flow.
UI (`src/components/interests/linked-berths-list.tsx`)
* Rows ordered with primary first; mooring number links to /berths/[id], with
area + a status pill (available/under_offer/sold) and a "Primary" chip.
* "Specifically pitching" Switch (writes `is_specific_interest`) with the
consequence text from §1: "This berth will appear as under interest on the
public map" / "This berth is hidden from the public map".
* "Mark in EOI bundle" Switch (writes `is_in_eoi_bundle`).
* "Set as primary" button when the row isn't primary - the existing
`upsertInterestBerth` helper demotes the prior primary in the same tx.
* "Bypass EOI for this berth" with reason textarea, ONLY rendered when the
parent interest's `eoiStatus === 'signed'`. Writes the bypass triple
(`eoi_bypass_reason`, `eoi_bypassed_by` = caller, `eoi_bypassed_at` = now);
also supports clearing.
* Remove-from-interest action gated by a confirmation dialog.
API (`src/app/api/v1/interests/[id]/berths/...`)
* `GET /` - list endpoint returning `listBerthsForInterest` plus the parent
interest's `eoiStatus` in `meta.eoiStatus` so the UI can decide whether to
show the bypass control.
* `PATCH /[berthId]` - partial update of the junction row's flags + bypass
fields. Server-side guard: rejects bypass writes when `eoiStatus !==
'signed'` (defence in depth - never trust the UI to gate this).
* `DELETE /[berthId]` - calls `removeInterestBerth`.
* The existing POST stays unchanged. All routes wrapped with
`withAuth(withPermission('interests', view|edit, ...))`. portId from ctx;
cross-port reads/writes return 404 for enumeration prevention (§14.10).
Service changes (`src/lib/services/interest-berths.service.ts`)
* `upsertInterestBerth` now accepts `eoiBypassReason` (tri-state: omit = no
change, non-empty = record, null = clear) and `eoiBypassedBy`. The bypass
triple moves as a unit, with `eoi_bypassed_at` stamped server-side.
* `listBerthsForInterest` now returns berth detail (area, status, dimensions)
alongside the junction row, typed as `InterestBerthWithDetails`.
Socket: added `interest:berthLinkUpdated` event for live UI refreshes.
Tests: 18 new integration tests in `tests/integration/api/interest-berths.test.ts`
covering happy paths, primary-demotion in same tx, bypass write/clear, the
"requires signed EOI" guard, cross-port 404s, missing-link 404s, empty-body
400, and viewer 403 through the permission gate.
|
||
|
Matt Ciaccio
|
6e3d910c76 |
refactor(interests): migrate callers to interest_berths junction + drop berth_id
Phase 2b of the berth-recommender refactor (plan §3.4). Every caller of the legacy `interests.berth_id` column now reads / writes through the `interest_berths` junction via the helper service introduced in Phase 2a; the column itself is dropped in a final migration. Service-layer changes - interests.service: filter `?berthId=X` becomes EXISTS-against-junction; list enrichment uses `getPrimaryBerthsForInterests`; create/update/ linkBerth/unlinkBerth all dispatch through the junction helpers, with createInterest's row insert + junction write sharing a single transaction. - clients / dashboard / report-generators / search: leftJoin chains pivot through `interest_berths` filtered by `is_primary=true`. - eoi-context / document-templates / berth-rules-engine / portal / record-export / queue worker: read primary via `getPrimaryBerth(...)`. - interest-scoring: berthLinked is now derived from any junction row count. - dedup/migration-apply + public interest route: write a primary junction row alongside the interest insert when a berth is provided. API contract preserved: list/detail responses still emit `berthId` and `berthMooringNumber`, derived from the primary junction row, so frontend consumers (interest-form, interest-detail-header) need no changes. Schema + migration - Drop `interestsRelations.berth` and `idx_interests_berth`. - Replace `berthsRelations.interests` with `interestBerths`. - Migration 0029_puzzling_romulus drops `interests.berth_id` + the index. - Tests that previously inserted `interests.berthId` now seed a primary junction row alongside the interest. Verified: vitest 995 passing (1 unrelated pre-existing flake in maintenance-cleanup.test.ts), tsc clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
Matt Ciaccio
|
ff92a08620 |
feat(db): m:m interest_berths junction + role flags
Introduces the multi-berth interest model from plan §3.1: a junction between interests and berths with three role flags so the same berth can be linked as the primary deal target, an EOI-bundle inclusion, or a "just exploring" link without conflating semantics. - 0028 schema migration creates interest_berths with the unique partial index "≤1 primary per interest", a unique compound on (interest_id, berth_id), and indexes for the public-map "under offer" lookup (where is_specific_interest=true). - Same migration adds desired_length_ft / desired_width_ft / desired_draft_ft to interests for the recommender. - Same migration runs the Phase 2 data migration: every interest with a non-null berth_id gets one junction row marked is_primary=true, is_specific_interest=true, and is_in_eoi_bundle = (eoi_status='signed'). Pre-flight check halts on dangling FKs (§14.3 critical case). - New service src/lib/services/interest-berths.service.ts owns reads + writes of the junction. getPrimaryBerth / getPrimaryBerthsForInterests feed list pages; upsertInterestBerth demotes the prior primary in the same transaction so the unique index is never violated. - interests.berth_id stays in place this commit so existing callers keep working; Phase 2b migrates them onto the helper service and a later migration drops the column. 53 dev rows seeded into the junction; tests still green at 996. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
