fix(audit-wave-11): file-lifecycle hardening — avatar leak + files FK
**file-lifecycle-auditor C1 — avatar replace leaks rows + blobs** `POST /api/v1/me/avatar` overwrote `userProfiles.avatarFileId` without reading or deleting the previous file id. Every "Replace photo" leaked one `files` row + one S3 blob, untethered (no client/yacht/company FK) and invisible to every existing UI sweep. Now captures the prior id BEFORE the UPDATE, then best-effort `deleteFile()` on the old row (handles ref-check + blob delete + audit) after the new id is committed. Failure is logged at warn — a stale blob shouldn't block the user from setting a new avatar. **file-lifecycle-auditor M1 — files.client_id missing ON DELETE** `files.client_id` was the only entity FK on the polymorphic `files` table that defaulted to `NO ACTION` (yacht_id + company_id were `SET NULL` per migration 0042). Any future bulk-client-delete that bypassed `hardDeleteClient`'s explicit FK-nullify pre-step would FK-violate. Migration `0059_files_client_id_onDelete_setnull.sql` brings it to parity; the explicit nullify in client-hard-delete is kept as defense in depth. Tests 1315/1315. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -5,8 +5,9 @@ import { withAuth } from '@/lib/api/helpers';
|
||||
import { db } from '@/lib/db';
|
||||
import { ports } from '@/lib/db/schema/ports';
|
||||
import { userProfiles } from '@/lib/db/schema/users';
|
||||
import { uploadFile } from '@/lib/services/files';
|
||||
import { deleteFile, uploadFile } from '@/lib/services/files';
|
||||
import { errorResponse, ValidationError } from '@/lib/errors';
|
||||
import { logger } from '@/lib/logger';
|
||||
|
||||
const MAX_AVATAR_BYTES = 2 * 1024 * 1024;
|
||||
|
||||
@@ -67,11 +68,40 @@ export const POST = withAuth(async (req, ctx) => {
|
||||
},
|
||||
);
|
||||
|
||||
// file-lifecycle-auditor C1: capture the prior avatar id BEFORE
|
||||
// overwriting so we can clean it up. Without this every "Replace
|
||||
// photo" leaked one files row + one S3 blob, untethered (no
|
||||
// client/yacht/company FK) and invisible to UI sweeps.
|
||||
const prior = await db.query.userProfiles.findFirst({
|
||||
where: eq(userProfiles.userId, ctx.userId),
|
||||
columns: { avatarFileId: true },
|
||||
});
|
||||
const priorAvatarId = prior?.avatarFileId ?? null;
|
||||
|
||||
await db
|
||||
.update(userProfiles)
|
||||
.set({ avatarFileId: record.id, updatedAt: new Date() })
|
||||
.where(eq(userProfiles.userId, ctx.userId));
|
||||
|
||||
if (priorAvatarId && priorAvatarId !== record.id) {
|
||||
// Best-effort delete — a stale-blob failure shouldn't fail the
|
||||
// new-avatar response. deleteFile handles ref-check + blob
|
||||
// delete + audit so a referenced file (somehow) is safe.
|
||||
try {
|
||||
await deleteFile(priorAvatarId, portId, {
|
||||
userId: ctx.userId,
|
||||
portId,
|
||||
ipAddress: ctx.ipAddress,
|
||||
userAgent: ctx.userAgent,
|
||||
});
|
||||
} catch (err) {
|
||||
logger.warn(
|
||||
{ err, priorAvatarId, userId: ctx.userId },
|
||||
'avatar replace: failed to clean up prior avatar file — orphan blob possible',
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
return NextResponse.json({ data: { avatarFileId: record.id } });
|
||||
} catch (error) {
|
||||
return errorResponse(error);
|
||||
|
||||
@@ -0,0 +1,18 @@
|
||||
-- file-lifecycle-auditor M1: `files.client_id` had no explicit
|
||||
-- `ON DELETE` (defaulted to NO ACTION), so any future bulk-client-delete
|
||||
-- or port-teardown that bypassed `hardDeleteClient` would FK-violate.
|
||||
-- Sister columns `files.yacht_id` and `files.company_id` were already
|
||||
-- SET NULL in migration 0042; bring `files.client_id` to parity so the
|
||||
-- "hard-delete is the only legal path" invariant becomes explicit.
|
||||
--
|
||||
-- The existing `client-hard-delete.service.ts` nullifies these FKs
|
||||
-- explicitly before the client DELETE; that pre-step is now redundant
|
||||
-- but harmless, so we don't remove it (defense in depth).
|
||||
|
||||
ALTER TABLE files
|
||||
DROP CONSTRAINT IF EXISTS files_client_id_clients_id_fk;
|
||||
|
||||
ALTER TABLE files
|
||||
ADD CONSTRAINT files_client_id_clients_id_fk
|
||||
FOREIGN KEY (client_id) REFERENCES clients(id)
|
||||
ON DELETE SET NULL;
|
||||
@@ -27,7 +27,7 @@ export const files = pgTable(
|
||||
portId: text('port_id')
|
||||
.notNull()
|
||||
.references(() => ports.id),
|
||||
clientId: text('client_id').references(() => clients.id),
|
||||
clientId: text('client_id').references(() => clients.id, { onDelete: 'set null' }),
|
||||
yachtId: text('yacht_id').references(() => yachts.id, { onDelete: 'set null' }),
|
||||
companyId: text('company_id').references(() => companies.id, { onDelete: 'set null' }),
|
||||
folderId: text('folder_id').references((): AnyPgColumn => documentFolders.id, {
|
||||
|
||||
Reference in New Issue
Block a user