fix: Robustheits-Luecken aus zweitem, unabhaengigem Sicherheitsaudit behoben
Ein zweiter, unabhaengiger Audit-Durchlauf (frischer Blick, keine Vorkenntnis meiner eigenen Fixes) bestaetigte alle bisherigen Fixes als korrekt verdrahtet und fand 3 LOW-severity Robustheitsluecken - keine davon war ausnutzbar, aber alle sind jetzt behoben: - HarvestsService.create/ExtractionsService.create pruefen jetzt, dass referenzierte colonyIds/harvestIds tatsaechlich existieren, bevor die Transaktion laeuft - vorher fuehrte eine unbekannte Id zu einem ungefangenen 500 mit rohem Prisma-Fehler statt einer sauberen 400. - Etiketten-Bildelemente durchlaufen jetzt denselben Magic-Byte-Check wie Avatar-/Dokument-Uploads, statt pdfkits Bildparser ungepruefte Bytes zu uebergeben. - Webhook-Secret-Redaktion im Audit-Log nutzt jetzt explizites delete statt sich auf Prismas JSON-Serialisierung zu verlassen, die undefined-Werte verwirft (fragil, falls sich die Serialisierung mal aendert). Co-Authored-By: Claude Sonnet 5 <[email protected]>
This commit is contained in:
1 parent
f57cffe6a8
commit
4bd7e9791a
7 files changed
+173
-7
No files matched your search
@@ -1,4 +1,4 @@
|
|||||||
import { NotFoundException } from '@nestjs/common';
|
import { BadRequestException, NotFoundException } from '@nestjs/common';
|
||||||
import { ExtractionsService } from './extractions.service';
|
import { ExtractionsService } from './extractions.service';
|
||||||
import { PrismaService } from '../prisma/prisma.service';
|
import { PrismaService } from '../prisma/prisma.service';
|
||||||
import { AuditLogService } from '../audit/audit-log.service';
|
import { AuditLogService } from '../audit/audit-log.service';
|
||||||
@@ -7,6 +7,7 @@ describe('ExtractionsService', () => {
|
|||||||
let service: ExtractionsService;
|
let service: ExtractionsService;
|
||||||
let prisma: {
|
let prisma: {
|
||||||
extraction: { findUnique: jest.Mock; update: jest.Mock };
|
extraction: { findUnique: jest.Mock; update: jest.Mock };
|
||||||
|
harvest: { findMany: jest.Mock };
|
||||||
$transaction: jest.Mock;
|
$transaction: jest.Mock;
|
||||||
};
|
};
|
||||||
let auditLog: { record: jest.Mock };
|
let auditLog: { record: jest.Mock };
|
||||||
@@ -24,6 +25,11 @@ describe('ExtractionsService', () => {
|
|||||||
};
|
};
|
||||||
prisma = {
|
prisma = {
|
||||||
extraction: { findUnique: jest.fn(), update: jest.fn() },
|
extraction: { findUnique: jest.fn(), update: jest.fn() },
|
||||||
|
harvest: {
|
||||||
|
findMany: jest
|
||||||
|
.fn()
|
||||||
|
.mockResolvedValue([{ id: 'harvest-1' }, { id: 'harvest-2' }]),
|
||||||
|
},
|
||||||
$transaction: jest.fn((callback: (txArg: typeof tx) => unknown) =>
|
$transaction: jest.fn((callback: (txArg: typeof tx) => unknown) =>
|
||||||
callback(tx),
|
callback(tx),
|
||||||
),
|
),
|
||||||
@@ -59,6 +65,18 @@ describe('ExtractionsService', () => {
|
|||||||
]);
|
]);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('create rejects an unknown harvest id instead of failing on the DB foreign-key constraint', async () => {
|
||||||
|
prisma.harvest.findMany.mockResolvedValue([{ id: 'harvest-1' }]);
|
||||||
|
|
||||||
|
await expect(
|
||||||
|
service.create(
|
||||||
|
{ date: '2026-07-05', harvestIds: ['harvest-1', 'missing-harvest'] },
|
||||||
|
'user-1',
|
||||||
|
),
|
||||||
|
).rejects.toBeInstanceOf(BadRequestException);
|
||||||
|
expect(prisma.$transaction).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
it('update writes an audit log entry with before and after data', async () => {
|
it('update writes an audit log entry with before and after data', async () => {
|
||||||
const updated = { ...extraction, notes: 'geändert' };
|
const updated = { ...extraction, notes: 'geändert' };
|
||||||
prisma.extraction.findUnique
|
prisma.extraction.findUnique
|
||||||
|
|||||||
@@ -1,4 +1,8 @@
|
|||||||
import { Injectable, NotFoundException } from '@nestjs/common';
|
import {
|
||||||
|
BadRequestException,
|
||||||
|
Injectable,
|
||||||
|
NotFoundException,
|
||||||
|
} from '@nestjs/common';
|
||||||
import { PrismaService } from '../prisma/prisma.service';
|
import { PrismaService } from '../prisma/prisma.service';
|
||||||
import { AuditLogService } from '../audit/audit-log.service';
|
import { AuditLogService } from '../audit/audit-log.service';
|
||||||
import { CreateExtractionDto } from './dto/create-extraction.dto';
|
import { CreateExtractionDto } from './dto/create-extraction.dto';
|
||||||
@@ -37,7 +41,22 @@ export class ExtractionsService {
|
|||||||
return extraction;
|
return extraction;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private async assertHarvestsExist(harvestIds: string[]): Promise<void> {
|
||||||
|
const found = await this.prisma.harvest.findMany({
|
||||||
|
where: { id: { in: harvestIds } },
|
||||||
|
select: { id: true },
|
||||||
|
});
|
||||||
|
const foundIds = new Set(found.map((harvest) => harvest.id));
|
||||||
|
const missing = harvestIds.filter((id) => !foundIds.has(id));
|
||||||
|
if (missing.length > 0) {
|
||||||
|
throw new BadRequestException(
|
||||||
|
`Unknown harvest id(s): ${missing.join(', ')}`,
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
async create(dto: CreateExtractionDto, actingUserId: string) {
|
async create(dto: CreateExtractionDto, actingUserId: string) {
|
||||||
|
await this.assertHarvestsExist(dto.harvestIds);
|
||||||
const extraction = await this.prisma.$transaction(async (tx) => {
|
const extraction = await this.prisma.$transaction(async (tx) => {
|
||||||
const created = await tx.extraction.create({
|
const created = await tx.extraction.create({
|
||||||
data: {
|
data: {
|
||||||
|
|||||||
@@ -1,4 +1,4 @@
|
|||||||
import { NotFoundException } from '@nestjs/common';
|
import { BadRequestException, NotFoundException } from '@nestjs/common';
|
||||||
import { HarvestsService } from './harvests.service';
|
import { HarvestsService } from './harvests.service';
|
||||||
import { PrismaService } from '../prisma/prisma.service';
|
import { PrismaService } from '../prisma/prisma.service';
|
||||||
import { AuditLogService } from '../audit/audit-log.service';
|
import { AuditLogService } from '../audit/audit-log.service';
|
||||||
@@ -8,6 +8,7 @@ describe('HarvestsService', () => {
|
|||||||
let service: HarvestsService;
|
let service: HarvestsService;
|
||||||
let prisma: {
|
let prisma: {
|
||||||
harvest: { findUnique: jest.Mock; update: jest.Mock };
|
harvest: { findUnique: jest.Mock; update: jest.Mock };
|
||||||
|
colony: { findMany: jest.Mock };
|
||||||
$transaction: jest.Mock;
|
$transaction: jest.Mock;
|
||||||
};
|
};
|
||||||
let auditLog: { record: jest.Mock };
|
let auditLog: { record: jest.Mock };
|
||||||
@@ -25,6 +26,11 @@ describe('HarvestsService', () => {
|
|||||||
};
|
};
|
||||||
prisma = {
|
prisma = {
|
||||||
harvest: { findUnique: jest.fn(), update: jest.fn() },
|
harvest: { findUnique: jest.fn(), update: jest.fn() },
|
||||||
|
colony: {
|
||||||
|
findMany: jest
|
||||||
|
.fn()
|
||||||
|
.mockResolvedValue([{ id: 'colony-1' }, { id: 'colony-2' }]),
|
||||||
|
},
|
||||||
$transaction: jest.fn((callback: (txArg: typeof tx) => unknown) =>
|
$transaction: jest.fn((callback: (txArg: typeof tx) => unknown) =>
|
||||||
callback(tx),
|
callback(tx),
|
||||||
),
|
),
|
||||||
@@ -66,6 +72,22 @@ describe('HarvestsService', () => {
|
|||||||
]);
|
]);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('create rejects an unknown colony id instead of failing on the DB foreign-key constraint', async () => {
|
||||||
|
prisma.colony.findMany.mockResolvedValue([{ id: 'colony-1' }]);
|
||||||
|
|
||||||
|
await expect(
|
||||||
|
service.create(
|
||||||
|
{
|
||||||
|
apiaryId: 'apiary-1',
|
||||||
|
date: '2026-07-01',
|
||||||
|
colonyIds: ['colony-1', 'missing-colony'],
|
||||||
|
},
|
||||||
|
'user-1',
|
||||||
|
),
|
||||||
|
).rejects.toBeInstanceOf(BadRequestException);
|
||||||
|
expect(prisma.$transaction).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
it('update writes an audit log entry with before and after data', async () => {
|
it('update writes an audit log entry with before and after data', async () => {
|
||||||
const updated = { ...harvest, notes: 'geändert' };
|
const updated = { ...harvest, notes: 'geändert' };
|
||||||
prisma.harvest.findUnique
|
prisma.harvest.findUnique
|
||||||
|
|||||||
@@ -1,4 +1,8 @@
|
|||||||
import { Injectable, NotFoundException } from '@nestjs/common';
|
import {
|
||||||
|
BadRequestException,
|
||||||
|
Injectable,
|
||||||
|
NotFoundException,
|
||||||
|
} from '@nestjs/common';
|
||||||
import { PrismaService } from '../prisma/prisma.service';
|
import { PrismaService } from '../prisma/prisma.service';
|
||||||
import { AuditLogService } from '../audit/audit-log.service';
|
import { AuditLogService } from '../audit/audit-log.service';
|
||||||
import { WebhooksService } from '../webhooks/webhooks.service';
|
import { WebhooksService } from '../webhooks/webhooks.service';
|
||||||
@@ -35,7 +39,22 @@ export class HarvestsService {
|
|||||||
return harvest;
|
return harvest;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private async assertColoniesExist(colonyIds: string[]): Promise<void> {
|
||||||
|
const found = await this.prisma.colony.findMany({
|
||||||
|
where: { id: { in: colonyIds } },
|
||||||
|
select: { id: true },
|
||||||
|
});
|
||||||
|
const foundIds = new Set(found.map((colony) => colony.id));
|
||||||
|
const missing = colonyIds.filter((id) => !foundIds.has(id));
|
||||||
|
if (missing.length > 0) {
|
||||||
|
throw new BadRequestException(
|
||||||
|
`Unknown colony id(s): ${missing.join(', ')}`,
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
async create(dto: CreateHarvestDto, actingUserId: string) {
|
async create(dto: CreateHarvestDto, actingUserId: string) {
|
||||||
|
await this.assertColoniesExist(dto.colonyIds);
|
||||||
const harvest = await this.prisma.$transaction(async (tx) => {
|
const harvest = await this.prisma.$transaction(async (tx) => {
|
||||||
const created = await tx.harvest.create({
|
const created = await tx.harvest.create({
|
||||||
data: {
|
data: {
|
||||||
|
|||||||
@@ -220,6 +220,75 @@ describe('LabelTemplatesService', () => {
|
|||||||
expect(pdf.toString('latin1')).toContain('/Image');
|
expect(pdf.toString('latin1')).toContain('/Image');
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('embeds an image element whose bytes match the declared PNG mime type', async () => {
|
||||||
|
// Minimal valid 1x1 transparent PNG, widely used as a test fixture.
|
||||||
|
const onePixelPng =
|
||||||
|
'iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNk+A8AAQUBAScY42YAAAAASUVORK5CYII=';
|
||||||
|
const imageElement: LabelElement = {
|
||||||
|
id: 'el-img',
|
||||||
|
type: 'image',
|
||||||
|
x: 5,
|
||||||
|
y: 5,
|
||||||
|
width: 10,
|
||||||
|
height: 10,
|
||||||
|
src: `data:image/png;base64,${onePixelPng}`,
|
||||||
|
};
|
||||||
|
prisma.labelTemplate.findUnique.mockResolvedValue({
|
||||||
|
...template,
|
||||||
|
elements: [imageElement],
|
||||||
|
});
|
||||||
|
prisma.bottlingBatch.findUnique.mockResolvedValue({
|
||||||
|
id: 'batch-1',
|
||||||
|
jarSize: 500,
|
||||||
|
jarCount: 1,
|
||||||
|
lotNumber: 'L-001',
|
||||||
|
date: new Date('2026-06-01T00:00:00.000Z'),
|
||||||
|
bestBeforeDate: new Date('2028-06-01T00:00:00.000Z'),
|
||||||
|
honeyBatch: { batchNumber: 'HB-001', honeyType: 'Bluetenhonig' },
|
||||||
|
product: { name: 'Glas 500g' },
|
||||||
|
});
|
||||||
|
|
||||||
|
const pdf = await service.renderLabelsPdf('template-1', 'batch-1');
|
||||||
|
|
||||||
|
// pdfkit always lists /ImageB /ImageC /ImageI in a page's ProcSet
|
||||||
|
// regardless of whether anything is actually embedded, so a plain
|
||||||
|
// "/Image" substring check would pass either way - /Subtype /Image is
|
||||||
|
// the actual embedded-XObject marker.
|
||||||
|
expect(pdf.toString('latin1')).toMatch(/\/Subtype\s*\/Image\b/);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('silently skips an image element whose bytes do not match the declared mime type', async () => {
|
||||||
|
const mislabeled = Buffer.from('not-a-png').toString('base64');
|
||||||
|
const imageElement: LabelElement = {
|
||||||
|
id: 'el-img',
|
||||||
|
type: 'image',
|
||||||
|
x: 5,
|
||||||
|
y: 5,
|
||||||
|
width: 10,
|
||||||
|
height: 10,
|
||||||
|
src: `data:image/png;base64,${mislabeled}`,
|
||||||
|
};
|
||||||
|
prisma.labelTemplate.findUnique.mockResolvedValue({
|
||||||
|
...template,
|
||||||
|
elements: [imageElement],
|
||||||
|
});
|
||||||
|
prisma.bottlingBatch.findUnique.mockResolvedValue({
|
||||||
|
id: 'batch-1',
|
||||||
|
jarSize: 500,
|
||||||
|
jarCount: 1,
|
||||||
|
lotNumber: 'L-001',
|
||||||
|
date: new Date('2026-06-01T00:00:00.000Z'),
|
||||||
|
bestBeforeDate: new Date('2028-06-01T00:00:00.000Z'),
|
||||||
|
honeyBatch: { batchNumber: 'HB-001', honeyType: 'Bluetenhonig' },
|
||||||
|
product: { name: 'Glas 500g' },
|
||||||
|
});
|
||||||
|
|
||||||
|
const pdf = await service.renderLabelsPdf('template-1', 'batch-1');
|
||||||
|
|
||||||
|
expect(pdf.subarray(0, 4).toString()).toBe('%PDF');
|
||||||
|
expect(pdf.toString('latin1')).not.toMatch(/\/Subtype\s*\/Image\b/);
|
||||||
|
});
|
||||||
|
|
||||||
function countPages(pdf: Buffer): number {
|
function countPages(pdf: Buffer): number {
|
||||||
const matches = pdf.toString('latin1').match(/\/Type\s*\/Page[^s]/g);
|
const matches = pdf.toString('latin1').match(/\/Type\s*\/Page[^s]/g);
|
||||||
return matches ? matches.length : 0;
|
return matches ? matches.length : 0;
|
||||||
|
|||||||
@@ -3,6 +3,7 @@ import PDFDocument from 'pdfkit';
|
|||||||
import * as QRCode from 'qrcode';
|
import * as QRCode from 'qrcode';
|
||||||
import { PrismaService } from '../prisma/prisma.service';
|
import { PrismaService } from '../prisma/prisma.service';
|
||||||
import { AuditLogService } from '../audit/audit-log.service';
|
import { AuditLogService } from '../audit/audit-log.service';
|
||||||
|
import { matchesFileSignature } from '../common/file-signature';
|
||||||
import { LabelTemplate } from '../../generated/prisma';
|
import { LabelTemplate } from '../../generated/prisma';
|
||||||
import { CreateLabelTemplateDto } from './dto/create-label-template.dto';
|
import { CreateLabelTemplateDto } from './dto/create-label-template.dto';
|
||||||
import { UpdateLabelTemplateDto } from './dto/update-label-template.dto';
|
import { UpdateLabelTemplateDto } from './dto/update-label-template.dto';
|
||||||
@@ -62,7 +63,13 @@ function dataUrlToBuffer(dataUrl: string): Buffer | null {
|
|||||||
if (!match) {
|
if (!match) {
|
||||||
return null;
|
return null;
|
||||||
}
|
}
|
||||||
return Buffer.from(match[2], 'base64');
|
const mimeType = match[1] === 'png' ? 'image/png' : 'image/jpeg';
|
||||||
|
const buffer = Buffer.from(match[2], 'base64');
|
||||||
|
// Same magic-byte check as every other upload path (avatar, documents) -
|
||||||
|
// a template's embedded image is only ever entered by a label-template
|
||||||
|
// editor (label-templates:create/update), but there's no reason to hand
|
||||||
|
// pdfkit's image parser bytes that don't match what they claim to be.
|
||||||
|
return matchesFileSignature(buffer, mimeType) ? buffer : null;
|
||||||
}
|
}
|
||||||
|
|
||||||
@Injectable()
|
@Injectable()
|
||||||
|
|||||||
@@ -52,7 +52,7 @@ export class WebhooksService {
|
|||||||
entityId: subscription.id,
|
entityId: subscription.id,
|
||||||
action: 'CREATE',
|
action: 'CREATE',
|
||||||
userId: actingUserId,
|
userId: actingUserId,
|
||||||
afterData: { ...subscription, secret: undefined },
|
afterData: this.omitSecret(subscription),
|
||||||
});
|
});
|
||||||
return subscription;
|
return subscription;
|
||||||
}
|
}
|
||||||
@@ -70,10 +70,22 @@ export class WebhooksService {
|
|||||||
entityId: id,
|
entityId: id,
|
||||||
action: 'DELETE',
|
action: 'DELETE',
|
||||||
userId: actingUserId,
|
userId: actingUserId,
|
||||||
beforeData: { ...subscription, secret: undefined },
|
beforeData: this.omitSecret(subscription),
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Explicit deletion rather than `{ ...subscription, secret: undefined }`:
|
||||||
|
// that pattern only works because Prisma's JSON serialization happens to
|
||||||
|
// drop undefined-valued keys today - an explicit delete doesn't depend on
|
||||||
|
// that being true forever.
|
||||||
|
private omitSecret(
|
||||||
|
subscription: WebhookSubscription,
|
||||||
|
): Record<string, unknown> {
|
||||||
|
const redacted: Partial<WebhookSubscription> = { ...subscription };
|
||||||
|
delete redacted.secret;
|
||||||
|
return redacted;
|
||||||
|
}
|
||||||
|
|
||||||
async dispatch(event: WebhookEvent, payload: unknown): Promise<void> {
|
async dispatch(event: WebhookEvent, payload: unknown): Promise<void> {
|
||||||
const subscriptions = await this.prisma.webhookSubscription.findMany({
|
const subscriptions = await this.prisma.webhookSubscription.findMany({
|
||||||
where: { active: true, events: { has: event } },
|
where: { active: true, events: { has: event } },
|
||||||
|
|||||||
Reference in new issue
Block a user