fix(security): brochures.service UPDATE/DELETE WHERE includes portId
audit-pass-#2 flagged that updateBrochure and archiveBrochure validate portId in their preceding SELECT but omit it from the subsequent UPDATE WHERE clause. Currently safe (the SELECT throws NotFoundError first), but a refactor that drops the SELECT or a TOCTOU race would silently allow a cross-tenant write. Defense-in-depth: add and(eq(id), eq(portId)) to both UPDATE WHERE clauses so the safety property doesn't depend on caller discipline. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -178,7 +178,7 @@ export async function updateBrochure(
|
|||||||
const [row] = await tx
|
const [row] = await tx
|
||||||
.update(brochures)
|
.update(brochures)
|
||||||
.set(updates)
|
.set(updates)
|
||||||
.where(eq(brochures.id, brochureId))
|
.where(and(eq(brochures.id, brochureId), eq(brochures.portId, portId)))
|
||||||
.returning();
|
.returning();
|
||||||
if (!row)
|
if (!row)
|
||||||
throw new CodedError('INSERT_RETURNING_EMPTY', {
|
throw new CodedError('INSERT_RETURNING_EMPTY', {
|
||||||
@@ -196,7 +196,7 @@ export async function archiveBrochure(portId: string, brochureId: string): Promi
|
|||||||
await db
|
await db
|
||||||
.update(brochures)
|
.update(brochures)
|
||||||
.set({ archivedAt: new Date(), isDefault: false })
|
.set({ archivedAt: new Date(), isDefault: false })
|
||||||
.where(eq(brochures.id, brochureId));
|
.where(and(eq(brochures.id, brochureId), eq(brochures.portId, portId)));
|
||||||
}
|
}
|
||||||
|
|
||||||
// ─── Versions ────────────────────────────────────────────────────────────────
|
// ─── Versions ────────────────────────────────────────────────────────────────
|
||||||
|
|||||||
Reference in New Issue
Block a user