fix(security): port-scope clientId/berthId/yachtId on interests + clientRelationships
Pass-6 findings — both MEDIUM cross-tenant FK injection. - interests.service: createInterest/updateInterest/linkBerth accepted clientId/berthId/yachtId from the request body without verifying the referenced row belongs to the caller's port. getInterestById joins clients/berths/yachtTags on these FKs without a port filter, so a port-A caller could splice a foreign-port id and surface that tenant's clientName, mooringNumber, or yacht ownership on read. New assertInterestFksInPort helper guards all three surfaces. - clients.service.createRelationship: accepted clientBId from the body without a port check; the relationship list endpoint joins clients without filtering by port, so the foreign client's name + email would render in the relationships tab. Now verifies clientBId belongs to portId and rejects self-relationships. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -13,7 +13,7 @@ import { yachts } from '@/lib/db/schema/yachts';
|
|||||||
import { berthReservations } from '@/lib/db/schema/reservations';
|
import { berthReservations } from '@/lib/db/schema/reservations';
|
||||||
import { tags } from '@/lib/db/schema/system';
|
import { tags } from '@/lib/db/schema/system';
|
||||||
import { createAuditLog, type AuditMeta } from '@/lib/audit';
|
import { createAuditLog, type AuditMeta } from '@/lib/audit';
|
||||||
import { NotFoundError } from '@/lib/errors';
|
import { NotFoundError, ValidationError } from '@/lib/errors';
|
||||||
import { isPortalEnabledForPort } from '@/lib/services/portal-auth.service';
|
import { isPortalEnabledForPort } from '@/lib/services/portal-auth.service';
|
||||||
import { setEntityTags } from '@/lib/services/entity-tags.helper';
|
import { setEntityTags } from '@/lib/services/entity-tags.helper';
|
||||||
import { emitToRoom } from '@/lib/socket/server';
|
import { emitToRoom } from '@/lib/socket/server';
|
||||||
@@ -663,11 +663,24 @@ export async function createRelationship(
|
|||||||
data: { clientBId: string; relationshipType: string; description?: string },
|
data: { clientBId: string; relationshipType: string; description?: string },
|
||||||
meta: AuditMeta,
|
meta: AuditMeta,
|
||||||
) {
|
) {
|
||||||
|
if (data.clientBId === clientId) {
|
||||||
|
throw new ValidationError('A client cannot have a relationship to themselves');
|
||||||
|
}
|
||||||
|
|
||||||
const client = await db.query.clients.findFirst({
|
const client = await db.query.clients.findFirst({
|
||||||
where: eq(clients.id, clientId),
|
where: eq(clients.id, clientId),
|
||||||
});
|
});
|
||||||
if (!client || client.portId !== portId) throw new NotFoundError('Client');
|
if (!client || client.portId !== portId) throw new NotFoundError('Client');
|
||||||
|
|
||||||
|
// Tenant scope: clientBId arrives from the request body. Without this check
|
||||||
|
// a port-A caller could splice a port-B client UUID onto their own client's
|
||||||
|
// relationship row; the GET handler joins clientRelationships → clients with
|
||||||
|
// no port filter and would surface the foreign client's name + email.
|
||||||
|
const otherClient = await db.query.clients.findFirst({
|
||||||
|
where: and(eq(clients.id, data.clientBId), eq(clients.portId, portId)),
|
||||||
|
});
|
||||||
|
if (!otherClient) throw new ValidationError('clientBId not found in this port');
|
||||||
|
|
||||||
const [rel] = await db
|
const [rel] = await db
|
||||||
.insert(clientRelationships)
|
.insert(clientRelationships)
|
||||||
.values({ portId, clientAId: clientId, ...data })
|
.values({ portId, clientAId: clientId, ...data })
|
||||||
|
|||||||
@@ -23,6 +23,49 @@ import type {
|
|||||||
|
|
||||||
// ─── Types ────────────────────────────────────────────────────────────────────
|
// ─── Types ────────────────────────────────────────────────────────────────────
|
||||||
|
|
||||||
|
// ─── Port-scope FK validator ─────────────────────────────────────────────────
|
||||||
|
|
||||||
|
// Tenant scope: every FK referenced from an interest body — clientId, berthId,
|
||||||
|
// and yachtId — must belong to the caller's port. Without this, a body-supplied
|
||||||
|
// foreign-port id would create an interest that joins through these FKs and
|
||||||
|
// surfaces foreign-tenant data on subsequent reads (clientName, berth mooring
|
||||||
|
// number, yacht ownership). assertYachtBelongsToClient still runs separately to
|
||||||
|
// enforce the additional ownership invariant.
|
||||||
|
async function assertInterestFksInPort(
|
||||||
|
portId: string,
|
||||||
|
fks: { clientId?: string | null; berthId?: string | null; yachtId?: string | null },
|
||||||
|
): Promise<void> {
|
||||||
|
const checks: Array<Promise<void>> = [];
|
||||||
|
if (fks.clientId) {
|
||||||
|
checks.push(
|
||||||
|
db.query.clients
|
||||||
|
.findFirst({ where: and(eq(clients.id, fks.clientId), eq(clients.portId, portId)) })
|
||||||
|
.then((row) => {
|
||||||
|
if (!row) throw new ValidationError('clientId not found in this port');
|
||||||
|
}),
|
||||||
|
);
|
||||||
|
}
|
||||||
|
if (fks.berthId) {
|
||||||
|
checks.push(
|
||||||
|
db.query.berths
|
||||||
|
.findFirst({ where: and(eq(berths.id, fks.berthId), eq(berths.portId, portId)) })
|
||||||
|
.then((row) => {
|
||||||
|
if (!row) throw new ValidationError('berthId not found in this port');
|
||||||
|
}),
|
||||||
|
);
|
||||||
|
}
|
||||||
|
if (fks.yachtId) {
|
||||||
|
checks.push(
|
||||||
|
db.query.yachts
|
||||||
|
.findFirst({ where: and(eq(yachts.id, fks.yachtId), eq(yachts.portId, portId)) })
|
||||||
|
.then((row) => {
|
||||||
|
if (!row) throw new ValidationError('yachtId not found in this port');
|
||||||
|
}),
|
||||||
|
);
|
||||||
|
}
|
||||||
|
await Promise.all(checks);
|
||||||
|
}
|
||||||
|
|
||||||
// ─── Yacht ownership validator ───────────────────────────────────────────────
|
// ─── Yacht ownership validator ───────────────────────────────────────────────
|
||||||
|
|
||||||
async function assertYachtBelongsToClient(
|
async function assertYachtBelongsToClient(
|
||||||
@@ -262,6 +305,12 @@ export async function getInterestById(id: string, portId: string) {
|
|||||||
// ─── Create ───────────────────────────────────────────────────────────────────
|
// ─── Create ───────────────────────────────────────────────────────────────────
|
||||||
|
|
||||||
export async function createInterest(portId: string, data: CreateInterestInput, meta: AuditMeta) {
|
export async function createInterest(portId: string, data: CreateInterestInput, meta: AuditMeta) {
|
||||||
|
await assertInterestFksInPort(portId, {
|
||||||
|
clientId: data.clientId,
|
||||||
|
berthId: data.berthId,
|
||||||
|
yachtId: data.yachtId,
|
||||||
|
});
|
||||||
|
|
||||||
if (data.yachtId) {
|
if (data.yachtId) {
|
||||||
await assertYachtBelongsToClient(portId, data.yachtId, data.clientId);
|
await assertYachtBelongsToClient(portId, data.yachtId, data.clientId);
|
||||||
}
|
}
|
||||||
@@ -338,6 +387,11 @@ export async function updateInterest(
|
|||||||
throw new NotFoundError('Interest');
|
throw new NotFoundError('Interest');
|
||||||
}
|
}
|
||||||
|
|
||||||
|
await assertInterestFksInPort(portId, {
|
||||||
|
berthId: data.berthId && data.berthId !== existing.berthId ? data.berthId : null,
|
||||||
|
yachtId: data.yachtId && data.yachtId !== existing.yachtId ? data.yachtId : null,
|
||||||
|
});
|
||||||
|
|
||||||
if (data.yachtId && data.yachtId !== existing.yachtId) {
|
if (data.yachtId && data.yachtId !== existing.yachtId) {
|
||||||
await assertYachtBelongsToClient(portId, data.yachtId, existing.clientId);
|
await assertYachtBelongsToClient(portId, data.yachtId, existing.clientId);
|
||||||
}
|
}
|
||||||
@@ -571,6 +625,8 @@ export async function linkBerth(id: string, portId: string, berthId: string, met
|
|||||||
throw new NotFoundError('Interest');
|
throw new NotFoundError('Interest');
|
||||||
}
|
}
|
||||||
|
|
||||||
|
await assertInterestFksInPort(portId, { berthId });
|
||||||
|
|
||||||
const [updated] = await db
|
const [updated] = await db
|
||||||
.update(interests)
|
.update(interests)
|
||||||
.set({ berthId, updatedAt: new Date() })
|
.set({ berthId, updatedAt: new Date() })
|
||||||
|
|||||||
74
tests/integration/client-relationship-port-isolation.test.ts
Normal file
74
tests/integration/client-relationship-port-isolation.test.ts
Normal file
@@ -0,0 +1,74 @@
|
|||||||
|
/**
|
||||||
|
* clients.service.createRelationship — tenant-FK validation tests.
|
||||||
|
*
|
||||||
|
* Covers the fix that requires both clientAId (the URL id) and clientBId
|
||||||
|
* (the body id) to belong to the caller's port. The list endpoint joins
|
||||||
|
* clientRelationships → clients without a port filter, so a foreign-port
|
||||||
|
* clientBId would surface that client's name in the relationship payload.
|
||||||
|
*/
|
||||||
|
import { describe, it, expect, beforeAll } from 'vitest';
|
||||||
|
|
||||||
|
describe('clients.service — createRelationship port isolation', () => {
|
||||||
|
let createRelationship: typeof import('@/lib/services/clients.service').createRelationship;
|
||||||
|
|
||||||
|
let makePort: typeof import('../helpers/factories').makePort;
|
||||||
|
let makeClient: typeof import('../helpers/factories').makeClient;
|
||||||
|
let makeAuditMeta: typeof import('../helpers/factories').makeAuditMeta;
|
||||||
|
|
||||||
|
beforeAll(async () => {
|
||||||
|
const svc = await import('@/lib/services/clients.service');
|
||||||
|
createRelationship = svc.createRelationship;
|
||||||
|
|
||||||
|
const factories = await import('../helpers/factories');
|
||||||
|
makePort = factories.makePort;
|
||||||
|
makeClient = factories.makeClient;
|
||||||
|
makeAuditMeta = factories.makeAuditMeta;
|
||||||
|
});
|
||||||
|
|
||||||
|
it('rejects createRelationship when clientBId belongs to a foreign port', async () => {
|
||||||
|
const portA = await makePort();
|
||||||
|
const portB = await makePort();
|
||||||
|
const localClient = await makeClient({ portId: portA.id });
|
||||||
|
const foreignClient = await makeClient({ portId: portB.id });
|
||||||
|
|
||||||
|
await expect(
|
||||||
|
createRelationship(
|
||||||
|
localClient.id,
|
||||||
|
portA.id,
|
||||||
|
{ clientBId: foreignClient.id, relationshipType: 'family' },
|
||||||
|
makeAuditMeta({ portId: portA.id }),
|
||||||
|
),
|
||||||
|
).rejects.toThrow(/clientBId not found in this port/);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('rejects creating a self-relationship', async () => {
|
||||||
|
const portA = await makePort();
|
||||||
|
const localClient = await makeClient({ portId: portA.id });
|
||||||
|
|
||||||
|
await expect(
|
||||||
|
createRelationship(
|
||||||
|
localClient.id,
|
||||||
|
portA.id,
|
||||||
|
{ clientBId: localClient.id, relationshipType: 'family' },
|
||||||
|
makeAuditMeta({ portId: portA.id }),
|
||||||
|
),
|
||||||
|
).rejects.toThrow(/cannot have a relationship to themselves/);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('accepts createRelationship when both clients are in the same port', async () => {
|
||||||
|
const portA = await makePort();
|
||||||
|
const clientA = await makeClient({ portId: portA.id });
|
||||||
|
const clientB = await makeClient({ portId: portA.id });
|
||||||
|
|
||||||
|
const rel = await createRelationship(
|
||||||
|
clientA.id,
|
||||||
|
portA.id,
|
||||||
|
{ clientBId: clientB.id, relationshipType: 'family' },
|
||||||
|
makeAuditMeta({ portId: portA.id }),
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(rel.clientAId).toBe(clientA.id);
|
||||||
|
expect(rel.clientBId).toBe(clientB.id);
|
||||||
|
expect(rel.portId).toBe(portA.id);
|
||||||
|
});
|
||||||
|
});
|
||||||
150
tests/integration/interests-port-fk-isolation.test.ts
Normal file
150
tests/integration/interests-port-fk-isolation.test.ts
Normal file
@@ -0,0 +1,150 @@
|
|||||||
|
/**
|
||||||
|
* interests.service tenant-FK validation tests.
|
||||||
|
*
|
||||||
|
* Covers the security fix that pins clientId/berthId/yachtId on interest
|
||||||
|
* create/update + linkBerth to the caller's port. Without these guards a
|
||||||
|
* port-A caller could splice a port-B FK onto their own interest and
|
||||||
|
* surface foreign-tenant data through the join surfaces in getInterestById.
|
||||||
|
*/
|
||||||
|
import { describe, it, expect, beforeAll } from 'vitest';
|
||||||
|
|
||||||
|
describe('interests.service — port-scope FK validation', () => {
|
||||||
|
let createInterest: typeof import('@/lib/services/interests.service').createInterest;
|
||||||
|
let updateInterest: typeof import('@/lib/services/interests.service').updateInterest;
|
||||||
|
let linkBerth: typeof import('@/lib/services/interests.service').linkBerth;
|
||||||
|
|
||||||
|
let makePort: typeof import('../helpers/factories').makePort;
|
||||||
|
let makeClient: typeof import('../helpers/factories').makeClient;
|
||||||
|
let makeBerth: typeof import('../helpers/factories').makeBerth;
|
||||||
|
let makeYacht: typeof import('../helpers/factories').makeYacht;
|
||||||
|
let makeAuditMeta: typeof import('../helpers/factories').makeAuditMeta;
|
||||||
|
|
||||||
|
beforeAll(async () => {
|
||||||
|
const svc = await import('@/lib/services/interests.service');
|
||||||
|
createInterest = svc.createInterest;
|
||||||
|
updateInterest = svc.updateInterest;
|
||||||
|
linkBerth = svc.linkBerth;
|
||||||
|
|
||||||
|
const factories = await import('../helpers/factories');
|
||||||
|
makePort = factories.makePort;
|
||||||
|
makeClient = factories.makeClient;
|
||||||
|
makeBerth = factories.makeBerth;
|
||||||
|
makeYacht = factories.makeYacht;
|
||||||
|
makeAuditMeta = factories.makeAuditMeta;
|
||||||
|
});
|
||||||
|
|
||||||
|
it('rejects createInterest with a foreign-port clientId', async () => {
|
||||||
|
const portA = await makePort();
|
||||||
|
const portB = await makePort();
|
||||||
|
const foreignClient = await makeClient({ portId: portB.id });
|
||||||
|
|
||||||
|
await expect(
|
||||||
|
createInterest(
|
||||||
|
portA.id,
|
||||||
|
{
|
||||||
|
clientId: foreignClient.id,
|
||||||
|
pipelineStage: 'open',
|
||||||
|
tagIds: [],
|
||||||
|
reminderEnabled: false,
|
||||||
|
},
|
||||||
|
makeAuditMeta({ portId: portA.id }),
|
||||||
|
),
|
||||||
|
).rejects.toThrow(/clientId not found in this port/);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('rejects createInterest with a foreign-port berthId', async () => {
|
||||||
|
const portA = await makePort();
|
||||||
|
const portB = await makePort();
|
||||||
|
const localClient = await makeClient({ portId: portA.id });
|
||||||
|
const foreignBerth = await makeBerth({ portId: portB.id });
|
||||||
|
|
||||||
|
await expect(
|
||||||
|
createInterest(
|
||||||
|
portA.id,
|
||||||
|
{
|
||||||
|
clientId: localClient.id,
|
||||||
|
berthId: foreignBerth.id,
|
||||||
|
pipelineStage: 'open',
|
||||||
|
tagIds: [],
|
||||||
|
reminderEnabled: false,
|
||||||
|
},
|
||||||
|
makeAuditMeta({ portId: portA.id }),
|
||||||
|
),
|
||||||
|
).rejects.toThrow(/berthId not found in this port/);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('rejects createInterest with a foreign-port yachtId', async () => {
|
||||||
|
const portA = await makePort();
|
||||||
|
const portB = await makePort();
|
||||||
|
const localClient = await makeClient({ portId: portA.id });
|
||||||
|
const foreignClient = await makeClient({ portId: portB.id });
|
||||||
|
const foreignYacht = await makeYacht({
|
||||||
|
portId: portB.id,
|
||||||
|
ownerType: 'client',
|
||||||
|
ownerId: foreignClient.id,
|
||||||
|
});
|
||||||
|
|
||||||
|
await expect(
|
||||||
|
createInterest(
|
||||||
|
portA.id,
|
||||||
|
{
|
||||||
|
clientId: localClient.id,
|
||||||
|
yachtId: foreignYacht.id,
|
||||||
|
pipelineStage: 'open',
|
||||||
|
tagIds: [],
|
||||||
|
reminderEnabled: false,
|
||||||
|
},
|
||||||
|
makeAuditMeta({ portId: portA.id }),
|
||||||
|
),
|
||||||
|
).rejects.toThrow(/yachtId not found in this port/);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('rejects updateInterest swapping in a foreign-port berthId', async () => {
|
||||||
|
const portA = await makePort();
|
||||||
|
const portB = await makePort();
|
||||||
|
const localClient = await makeClient({ portId: portA.id });
|
||||||
|
const foreignBerth = await makeBerth({ portId: portB.id });
|
||||||
|
|
||||||
|
const interest = await createInterest(
|
||||||
|
portA.id,
|
||||||
|
{
|
||||||
|
clientId: localClient.id,
|
||||||
|
pipelineStage: 'open',
|
||||||
|
tagIds: [],
|
||||||
|
reminderEnabled: false,
|
||||||
|
},
|
||||||
|
makeAuditMeta({ portId: portA.id }),
|
||||||
|
);
|
||||||
|
|
||||||
|
await expect(
|
||||||
|
updateInterest(
|
||||||
|
interest.id,
|
||||||
|
portA.id,
|
||||||
|
{ berthId: foreignBerth.id },
|
||||||
|
makeAuditMeta({ portId: portA.id }),
|
||||||
|
),
|
||||||
|
).rejects.toThrow(/berthId not found in this port/);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('rejects linkBerth with a foreign-port berthId', async () => {
|
||||||
|
const portA = await makePort();
|
||||||
|
const portB = await makePort();
|
||||||
|
const localClient = await makeClient({ portId: portA.id });
|
||||||
|
const foreignBerth = await makeBerth({ portId: portB.id });
|
||||||
|
|
||||||
|
const interest = await createInterest(
|
||||||
|
portA.id,
|
||||||
|
{
|
||||||
|
clientId: localClient.id,
|
||||||
|
pipelineStage: 'open',
|
||||||
|
tagIds: [],
|
||||||
|
reminderEnabled: false,
|
||||||
|
},
|
||||||
|
makeAuditMeta({ portId: portA.id }),
|
||||||
|
);
|
||||||
|
|
||||||
|
await expect(
|
||||||
|
linkBerth(interest.id, portA.id, foreignBerth.id, makeAuditMeta({ portId: portA.id })),
|
||||||
|
).rejects.toThrow(/berthId not found in this port/);
|
||||||
|
});
|
||||||
|
});
|
||||||
Reference in New Issue
Block a user