From 74970f6073203021118743c598340ec87868befd Mon Sep 17 00:00:00 2001 From: Claude Fable 5 Date: Fri, 31 Jul 2026 04:32:29 +0200 Subject: [PATCH] #199: SHA-256 integrity hashes for attachments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every upload stores the SHA-256 of its bytes, computed from the in-memory buffer that is written — never by re-reading disk. Every download re-hashes the stored object BEFORE the first byte leaves (memory bounded by the max_file_bytes quota that gated the upload) and fails closed on mismatch with attachment_integrity_failure; the mismatch lands in the audit trail as file.integrity_failed with both hashes. Detection of payload manipulation is the one integrity duty par. 52 VSA leaves with the application — only it knows what the file should be. Pre-#199 rows are hashed by a bounded, idempotent backfill that rides the existing nightly orphan-file-sweep job (no new scheduler job, job fence untouched); unreadable files are logged and retried, never silently skipped, and null-hash rows are served unverified only until the backfill reaches them. Operator runbook note in security.md (restore from backup, re-download, audit entry carries both hashes). Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_0168Ph5uBmHm8X28CSVpbpnJ --- .../migration.sql | 5 + apps/api/prisma/schema.prisma | 5 + .../files/attachment-integrity.e2e.db.test.ts | 147 ++++++++++++++++++ apps/api/src/files/file-storage.service.ts | 9 +- apps/api/src/files/files.module.ts | 5 + apps/api/src/files/files.service.ts | 79 +++++++++- docs/architecture/operations.md | 6 +- docs/architecture/security.md | 14 ++ docs/vs-nfd/20-massnahmenplan.md | 2 +- 9 files changed, 266 insertions(+), 6 deletions(-) create mode 100644 apps/api/prisma/migrations/20260731090000_attachment_sha256/migration.sql create mode 100644 apps/api/src/files/attachment-integrity.e2e.db.test.ts diff --git a/apps/api/prisma/migrations/20260731090000_attachment_sha256/migration.sql b/apps/api/prisma/migrations/20260731090000_attachment_sha256/migration.sql new file mode 100644 index 0000000..12449a4 --- /dev/null +++ b/apps/api/prisma/migrations/20260731090000_attachment_sha256/migration.sql @@ -0,0 +1,5 @@ +-- #199: integrity hash for uploaded files. New uploads store the SHA-256 of +-- their bytes at write time; existing rows are hashed by the nightly +-- backfill (part of the orphan-file-sweep job), which reads the uploads +-- volume — something this SQL migration cannot do. +ALTER TABLE "attachments" ADD COLUMN "sha256" TEXT; diff --git a/apps/api/prisma/schema.prisma b/apps/api/prisma/schema.prisma index 98d7ccd..b8043e6 100644 --- a/apps/api/prisma/schema.prisma +++ b/apps/api/prisma/schema.prisma @@ -550,6 +550,11 @@ model Attachment { sizeBytes Int @map("size_bytes") storagePath String @map("storage_path") uploadedBy String @map("uploaded_by") + /// SHA-256 (hex) of the stored bytes (issue #199), computed from the + /// in-memory upload buffer as it is written — never by re-reading disk. + /// Downloads verify against it and fail closed on mismatch. Null only + /// for rows that predate #199 until the nightly backfill hashes them. + sha256 String? createdAt DateTime @default(now()) @map("created_at") pond Pond @relation(fields: [pondId], references: [id]) diff --git a/apps/api/src/files/attachment-integrity.e2e.db.test.ts b/apps/api/src/files/attachment-integrity.e2e.db.test.ts new file mode 100644 index 0000000..0f2fcc2 --- /dev/null +++ b/apps/api/src/files/attachment-integrity.e2e.db.test.ts @@ -0,0 +1,147 @@ +import { createHash } from 'node:crypto'; + +import { INestApplication, InternalServerErrorException } from '@nestjs/common'; +import { PrismaClient, User } from '@prisma/client'; +import request from 'supertest'; +import { afterAll, beforeAll, describe, expect, it } from 'vitest'; + +import { AuthTokensService } from '../auth/auth-tokens.service'; +import { createTestApp } from '../testing/test-app'; +import { createTestPrisma, hasTestDb, uniqueSuffix } from '../testing/test-db'; +import { UsersService } from '../users/users.service'; + +import { FileStorageService } from './file-storage.service'; +import { FilesService } from './files.service'; + +const PNG_SIGNATURE = Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a]); +const pngBuffer = (payload: string): Buffer => Buffer.concat([PNG_SIGNATURE, Buffer.from(payload)]); +const sha256 = (buffer: Buffer): string => createHash('sha256').update(buffer).digest('hex'); + +/** + * Attachment integrity (issue #199): uploads store the SHA-256 of the + * written bytes, downloads verify it and fail closed (audited) on mismatch, + * and the nightly backfill hashes pre-#199 rows idempotently, reporting + * unreadable files instead of skipping them. + */ +describe.skipIf(!hasTestDb)('attachment integrity (e2e, issue #199)', () => { + let app: INestApplication; + let prisma: PrismaClient; + let files: FilesService; + let storage: FileStorageService; + let user: User; + let pondId: string; + const suffix = uniqueSuffix(); + + async function uploadPng(payload: string): Promise<{ id: string; bytes: Buffer }> { + const bytes = pngBuffer(payload); + const view = await files.upload(user, pondId, { + buffer: bytes, + size: bytes.length, + originalname: `${payload}.png`, + }); + return { id: view.id, bytes }; + } + + beforeAll(async () => { + prisma = createTestPrisma(); + app = await createTestApp(); + files = app.get(FilesService); + storage = app.get(FileStorageService); + + const users = app.get(UsersService); + user = await users.createUser({ + username: `ines-integrity-${suffix}`, + email: `ines-integrity-${suffix}@example.org`, + displayName: `Ines Integrity ${suffix}`, + password: 'jedes byte bleibt wie es war 1', + locale: 'en', + }); + // Verification via the endpoint (not markEmailVerified) because only the + // endpoint creates the personal pond the uploads go into. + const token = await app.get(AuthTokensService).issue(user.id, 'EMAIL_VERIFICATION', 600); + await request(app.getHttpServer()) + .post('/api/v1/auth/verify-email') + .send({ token }) + .expect(204); + const pond = await prisma.pond.findFirstOrThrow({ where: { ownerId: user.id } }); + pondId = pond.id; + }); + + afterAll(async () => { + await prisma.auditEntry.deleteMany({ + where: { action: 'file.integrity_failed', details: { path: ['pondId'], equals: pondId } }, + }); + await prisma.attachment.deleteMany({ where: { pondId } }); + const where = { pond: { owner: { username: { contains: suffix } } } }; + await prisma.roleGrant.deleteMany({ where }); + await prisma.pond.deleteMany({ where: { owner: { username: { contains: suffix } } } }); + await prisma.user.deleteMany({ where: { username: { contains: suffix } } }); + await prisma.$disconnect(); + await app.close(); + }); + + it('stores the hash of the written bytes at upload', async () => { + const { id, bytes } = await uploadPng('honest-upload'); + const row = await prisma.attachment.findUniqueOrThrow({ where: { id } }); + expect(row.sha256).toBe(sha256(bytes)); + }); + + it('serves an intact file and fails closed, audited, on a tampered one', async () => { + const { id, bytes } = await uploadPng('will-be-tampered'); + + // Intact: the download succeeds and streams the exact bytes. + const intact = await files.download(null, id); + const chunks: Buffer[] = []; + for await (const chunk of intact.stream) chunks.push(chunk as Buffer); + expect(Buffer.concat(chunks).equals(bytes)).toBe(true); + + // Tampered on disk (row untouched): fail closed with the dedicated code. + await storage.save(pondId, id, pngBuffer('evil-replacement')); + const failure = await files.download(null, id).catch((error: unknown) => error); + expect(failure).toBeInstanceOf(InternalServerErrorException); + expect((failure as InternalServerErrorException).getResponse()).toMatchObject({ + code: 'attachment_integrity_failure', + }); + + // The mismatch is on the audit trail with both hashes. + const audit = await prisma.auditEntry.findFirst({ + where: { action: 'file.integrity_failed', targetId: id }, + }); + expect(audit).not.toBeNull(); + expect(audit!.details).toMatchObject({ + expected: sha256(bytes), + actual: sha256(pngBuffer('evil-replacement')), + }); + }); + + it('backfills missing hashes idempotently and reports unreadable files', async () => { + const readable = await uploadPng('backfill-me'); + const unreadable = await uploadPng('bytes-will-vanish'); + await prisma.attachment.updateMany({ + where: { id: { in: [readable.id, unreadable.id] } }, + data: { sha256: null }, + }); + await storage.delete(pondId, unreadable.id); + + // A null-hash row is served unverified (pre-#199 status quo). + const unverified = await files.download(null, readable.id); + expect(unverified.attachment.sha256).toBeNull(); + + const first = await files.backfillHashes(); + expect(first.hashed).toBeGreaterThanOrEqual(1); + expect(first.unreadable).toBeGreaterThanOrEqual(1); + const rehashed = await prisma.attachment.findUniqueOrThrow({ where: { id: readable.id } }); + expect(rehashed.sha256).toBe(sha256(readable.bytes)); + + // The unreadable row keeps its null hash — reported, retried next run, + // never silently marked done. + const vanished = await prisma.attachment.findUniqueOrThrow({ where: { id: unreadable.id } }); + expect(vanished.sha256).toBeNull(); + + // Idempotent: a second run finds nothing new to hash here. + const second = await files.backfillHashes(); + const third = await prisma.attachment.findUniqueOrThrow({ where: { id: readable.id } }); + expect(third.sha256).toBe(sha256(readable.bytes)); + expect(second.unreadable).toBeGreaterThanOrEqual(1); + }); +}); diff --git a/apps/api/src/files/file-storage.service.ts b/apps/api/src/files/file-storage.service.ts index 3a7cba7..ddece70 100644 --- a/apps/api/src/files/file-storage.service.ts +++ b/apps/api/src/files/file-storage.service.ts @@ -1,5 +1,5 @@ import { createReadStream } from 'node:fs'; -import { access, mkdir, readdir, rm, stat, writeFile } from 'node:fs/promises'; +import { access, mkdir, readdir, readFile, rm, stat, writeFile } from 'node:fs/promises'; import { join } from 'node:path'; import type { Readable } from 'node:stream'; @@ -30,6 +30,13 @@ export class FileStorageService { return createReadStream(this.pathFor(pondId, fileId)); } + /** The complete stored bytes. Used where the caller must see the whole + * object before serving a single byte of it — integrity verification + * (issue #199) cannot work on a stream that is already leaving. */ + read(pondId: string, fileId: string): Promise { + return readFile(this.pathFor(pondId, fileId)); + } + /** Whether the file's bytes are actually on disk. Used by the pond export to * skip an attachment whose bytes are missing (data drift) rather than crash * the archive stream (issue #65). */ diff --git a/apps/api/src/files/files.module.ts b/apps/api/src/files/files.module.ts index fcddffe..2599883 100644 --- a/apps/api/src/files/files.module.ts +++ b/apps/api/src/files/files.module.ts @@ -24,6 +24,7 @@ export class FilesModule implements OnModuleInit { constructor( private readonly scheduler: SchedulerService, private readonly sweep: OrphanSweepService, + private readonly files: FilesService, ) {} onModuleInit(): void { @@ -32,6 +33,10 @@ export class FilesModule implements OnModuleInit { cadenceSeconds: ORPHAN_SWEEP_CADENCE_SECONDS, run: async () => { await this.sweep.sweep(); + // Same nightly volume walk, same domain: hash rows that predate + // #199 until none remain (idempotent, bounded batch) — a separate + // scheduled job would outlive its purpose. + await this.files.backfillHashes(); }, }); } diff --git a/apps/api/src/files/files.service.ts b/apps/api/src/files/files.service.ts index 444f945..5e502c3 100644 --- a/apps/api/src/files/files.service.ts +++ b/apps/api/src/files/files.service.ts @@ -1,9 +1,10 @@ -import { randomUUID } from 'node:crypto'; -import type { Readable } from 'node:stream'; +import { createHash, randomUUID } from 'node:crypto'; +import { Readable } from 'node:stream'; import { BadRequestException, Injectable, + InternalServerErrorException, NotFoundException, PayloadTooLargeException, } from '@nestjs/common'; @@ -19,6 +20,7 @@ import { import { Attachment, User } from '@prisma/client'; import { PinoLogger } from 'nestjs-pino'; +import { AuditService } from '../audit/audit.service'; import { PrismaService } from '../prisma/prisma.service'; import { QuotaService } from '../quotas/quota.service'; import { InstanceSettingsService } from '../settings/instance-settings.service'; @@ -51,6 +53,7 @@ export class FilesService { private readonly quotas: QuotaService, private readonly storage: FileStorageService, private readonly settings: InstanceSettingsService, + private readonly audit: AuditService, private readonly logger: PinoLogger, ) { this.logger.setContext(FilesService.name); @@ -153,6 +156,9 @@ export class FilesService { sizeBytes, storagePath: `${pond.id}/${id}`, uploadedBy: user.id, + // Integrity hash (issue #199): computed from the exact in-memory + // bytes that were just written — never by re-reading the disk. + sha256: createHash('sha256').update(resolved.buffer).digest('hex'), }, }); this.logger.info( @@ -195,16 +201,83 @@ export class FilesService { return this.upload(user, page.pondId, file, page.id); } + /** + * Serve an attachment, verifying its integrity first (issue #199): the + * whole object is read and hashed BEFORE the first byte leaves — a stream + * cannot be un-sent, so verification must precede serving. Memory is + * bounded by the `max_file_bytes` quota that gated the upload. A mismatch + * fails closed with its own error code and lands in the audit trail (a + * security event, not content activity); the operator's move is a restore + * from backup (runbook). Rows that predate #199 (sha256 still null until + * the nightly backfill reaches them) are served unverified — that is the + * pre-#199 status quo, not a downgrade. + */ async download(_user: User | null, id: string): Promise { const attachment = await this.prisma.attachment.findFirst({ where: { id } }); if (!attachment) throw new NotFoundException(); + const buffer = await this.storage.read(attachment.pondId, attachment.id).catch(() => null); + if (!buffer) throw new NotFoundException(); + if (attachment.sha256) { + const actual = createHash('sha256').update(buffer).digest('hex'); + if (actual !== attachment.sha256) { + await this.audit.record({ + action: 'file.integrity_failed', + targetType: 'attachment', + targetId: attachment.id, + details: { pondId: attachment.pondId, expected: attachment.sha256, actual }, + }); + throw new InternalServerErrorException({ code: 'attachment_integrity_failure' }); + } + } return { attachment, - stream: this.storage.createReadStream(attachment.pondId, attachment.id), + stream: Readable.from(buffer), inline: isImageMimeType(attachment.mimeType), }; } + /** + * Hash attachments that predate #199 (sha256 null), a bounded batch per + * nightly run until none remain — idempotent by construction (hashed rows + * stop matching). An unreadable file is reported (log + count) and left + * null so the next run retries it; the orphan sweep is the mechanism that + * eventually explains truly missing bytes. + */ + async backfillHashes(limit = 1000): Promise<{ hashed: number; unreadable: number }> { + const rows = await this.prisma.attachment.findMany({ + where: { sha256: null }, + select: { id: true, pondId: true }, + take: limit, + }); + let hashed = 0; + let unreadable = 0; + for (const row of rows) { + let buffer: Buffer; + try { + buffer = await this.storage.read(row.pondId, row.id); + } catch (error) { + unreadable += 1; + this.logger.error( + { attachmentId: row.id, pondId: row.pondId, err: error }, + 'attachment unreadable during hash backfill; will retry next run', + ); + continue; + } + await this.prisma.attachment.update({ + where: { id: row.id }, + data: { sha256: createHash('sha256').update(buffer).digest('hex') }, + }); + hashed += 1; + } + if (rows.length > 0) { + this.logger.info( + { hashed, unreadable, batch: rows.length, batchLimit: limit }, + 'audit: attachment hash backfill progress', + ); + } + return { hashed, unreadable }; + } + /** Attachments linked to a page, for its attachments section (#61). */ async listForPage(pageId: string): Promise { const rows = await this.prisma.attachment.findMany({ diff --git a/docs/architecture/operations.md b/docs/architecture/operations.md index fa78c7c..ad06098 100644 --- a/docs/architecture/operations.md +++ b/docs/architecture/operations.md @@ -123,7 +123,11 @@ page content no longer embeds them are deliberately NOT auto-deleted: the page attachments panel lists them as user-managed objects (insert is optional there), so cleanup of those is a human decision in the panel or the pond file manager. Attachment deletion is hard everywhere — the -unused `deleted_at` column was removed with #194. +unused `deleted_at` column was removed with #194. Since #199 the same +nightly run also backfills SHA-256 integrity hashes for attachments that +predate the column (bounded batch per night, idempotent; unreadable +files are logged and retried, see security.md §Content & upload +security). Conversion payload prune (issue #233): daily. Nulls the raw `input` and `result` bytes of import/export conversion jobs that finished (succeeded diff --git a/docs/architecture/security.md b/docs/architecture/security.md index fb36f33..783851a 100644 --- a/docs/architecture/security.md +++ b/docs/architecture/security.md @@ -64,6 +64,20 @@ or sloppy plugin authors, compromised dependencies. - App CSP (strict): `default-src 'self'`; `font-src 'self'` (ADR 0016); no third-party origins at all — the GDPR posture is "zero external requests". +- **Attachment integrity (issue #199)**: every upload stores the SHA-256 + of its bytes, computed from the in-memory buffer as it is written (never + by re-reading disk). Every download re-hashes the stored object BEFORE + the first byte leaves and fails closed on mismatch with + `attachment_integrity_failure` (HTTP 500); the mismatch is recorded in + the audit trail (`file.integrity_failed`). §52 VSA leaves detecting + manipulation of the application's own payloads to the application — + only it knows what the file should be. Operator response to a + verification failure: treat the object as tampered/corrupt, restore the + affected file from backup (restore runbook), then re-download to + confirm; the audit entry carries both hashes for the report. + Pre-existing rows are hashed by the nightly backfill (part of the + orphan-file-sweep job) and served unverified only until it reaches + them; unreadable files are logged and retried, never silently skipped. - The full-text index holds **no trashed content** (issue #195): trashing a page or pond clears the affected `search_vector`s, restore rebuilds them, `reindexAll` converges to the same invariant, and a one-off diff --git a/docs/vs-nfd/20-massnahmenplan.md b/docs/vs-nfd/20-massnahmenplan.md index 13f8162..61047d5 100644 --- a/docs/vs-nfd/20-massnahmenplan.md +++ b/docs/vs-nfd/20-massnahmenplan.md @@ -107,7 +107,7 @@ chain`_ - [x] **Security-Header** (helmet), CORS explizit restriktiv · 1 AT · #197 - [x] **SBOM in CI** (CycloneDX/syft) + Lizenzreport als Artefakt · 1–2 AT · #202 - [x] `deploy/compose/.env` prüfen, Beispieldatei statt Realdatei · 0,5 AT · #198 -- [ ] **Attachment-Integritätshashes** · +2–3 AT · #199 ⟵ neu aus Roadmap +- [x] **Attachment-Integritätshashes** · +2–3 AT · #199 ⟵ neu aus Roadmap SHA-256-Spalte, Berechnung beim Upload, Prüfung beim Download, Backfill-Migration. Nebennutzen: Orphan-Sweep, Dedup, Backup-Verifikation. - [ ] **Plugins hart abschaltbar** (`plugins.enabled = false`) · +2 AT · #200 ⟵ neu